ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: "Eregon (Benoit Daloze) via ruby-core" <ruby-core@ml.ruby-lang.org>
To: ruby-core@ml.ruby-lang.org
Cc: "Eregon (Benoit Daloze)" <noreply@ruby-lang.org>
Subject: [ruby-core:117596] [Ruby master Misc#20434] Deprecate encoding-related regular expression modifiers
Date: Thu, 18 Apr 2024 11:41:15 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-108001.20240418114114.10206@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-20434.20240417155807.10206@ruby-lang.org

Issue #20434 has been updated by Eregon (Benoit Daloze).


This seems a good simplification to me, I think the semantics of these encoding modifiers are confusing to most Rubyists.

I wouldn't be worried too much about length of the replacement, because `/.../s`/`/.../e` are likely very rare (using file encoding seems a good replacement for those).
`/.../u` seems redundant with the default source encoding, so the `u` can likely just be removed in most cases.

I'm not so sure `/.../n`, that may be more frequent.

Methods to convert an existing Regexp from one encoding to another feel suboptiomal, because that will cause an extra Regexp instance and creating a Regexp is not cheap due to many checks, allocations, and even compilation (AFAIK eager in CRuby at least).

So I think the existing `Regexp.new("a".dup.force_encoding(Encoding::WINDOWS_31J))` is good enough.
And since this would address a deprecation, it seems very important that the code also works on older Ruby versions.

(I'm all for `String#{encoded,with_encoding}` but it seems best to propose that as a separate ticket)

I would be interested to have a good textual description in #20406 of how the encoding of a Regexp is computed currently, it seems quite complex, but having it in text would allow to reason more easily about it.
Maybe we could simplify it while remaining compatible (i.e. the specific value of Regexp#encoding matters not so much, what matters is a Regexp can still be matched against Strings of various encoding like it could before).

----------------------------------------
Misc #20434: Deprecate encoding-related regular expression modifiers
https://bugs.ruby-lang.org/issues/20434#change-108001

* Author: kddnewton (Kevin Newton)
* Status: Open
----------------------------------------
This is a follow-up to @duerst's comment here: https://bugs.ruby-lang.org/issues/20406#note-6.

As noted in the other issue, there are many encodings that factor in to how a regular expression operates. This includes:

* The encoding of the file
* The encoding of the string parts within the regular expression
* The regular expression encoding modifiers
* The encoding of the string being matched

At the time the modifiers were introduced, I believe the modifiers may have been the only (??) encoding that factored in here. At this point, however, they can lead to quite a bit of confusion, as noted in the other ticket.

I would like to propose to deprecate the regular expression encoding modifiers. Instead, we could suggest in a warning to instead create a regular expression with an encoded string. For example, when we find:

```ruby
/\x81\x40/s
```

we would instead suggest:

```ruby
::Regexp.new(::String.new("\x81\x40", encoding: "Windows-31J"))
```

or equivalent. As a migration path, we could do the following:

1. Emit a warning to change to the suggested expression
2. Change the compiler to compile to the suggested expression when those flags are found
3. Remove support for the flags

Step 2 may be unnecessary depending on how long of a timeline we would like to provide. To be clear, I'm not advocating for any particular timeline, and would be fine with this being multiple years/versions to give plenty of time for people to migrate. But I do think this would be a good change to eliminate confusion about the interaction between the four different encodings at play.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

  parent reply	other threads:[~2024-04-18 11:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-17 15:58 [ruby-core:117568] [Ruby master Misc#20434] Deprecate regular expression modifiers kddnewton (Kevin Newton) via ruby-core
2024-04-18  0:23 ` [ruby-core:117581] [Ruby master Misc#20434] Deprecate encoding-releated " shyouhei (Shyouhei Urabe) via ruby-core
2024-04-18  6:22 ` [ruby-core:117591] " duerst via ruby-core
2024-04-18  9:23 ` [ruby-core:117595] " byroot (Jean Boussier) via ruby-core
2024-04-18 11:41 ` Eregon (Benoit Daloze) via ruby-core [this message]
2024-04-18 18:50 ` [ruby-core:117600] [Ruby master Misc#20434] Deprecate encoding-related " kddnewton (Kevin Newton) via ruby-core

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-108001.20240418114114.10206@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    --cc=noreply@ruby-lang.org \
    --cc=ruby-core@ml.ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).