From fe4641a56dcbab8a9e1e63ab4f21fd02f2c92a1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 8 May 2024 11:41:34 +0200 Subject: [PATCH 1/2] Fix `Regex#inspect` with non-literal-compatible options --- spec/std/regex_spec.cr | 28 +++++++++++++++++++++------- src/regex.cr | 42 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/spec/std/regex_spec.cr b/spec/std/regex_spec.cr index 310f94ffc9b4..13d301987c56 100644 --- a/spec/std/regex_spec.cr +++ b/spec/std/regex_spec.cr @@ -449,15 +449,29 @@ describe "Regex" do end describe "#inspect" do - it "with options" do - /foo/.inspect.should eq("/foo/") - /foo/im.inspect.should eq("/foo/im") - /foo/imx.inspect.should eq("/foo/imx") + context "with literal-compatible options" do + it "prints flags" do + /foo/.inspect.should eq("/foo/") + /foo/im.inspect.should eq("/foo/im") + /foo/imx.inspect.should eq("/foo/imx") + end + + it "escapes" do + %r(/).inspect.should eq("/\\//") + %r(\/).inspect.should eq("/\\//") + end end - it "escapes" do - %r(/).inspect.should eq("/\\//") - %r(\/).inspect.should eq("/\\//") + context "with non-literal-compatible options" do + it "prints flags" do + Regex.new("foo", :anchored).inspect.should eq %(Regex.new("foo", Regex::Options::ANCHORED)) + Regex.new("foo", :no_utf_check).inspect.should eq %(Regex.new("foo", Regex::Options::NO_UTF8_CHECK)) + Regex.new("foo", Regex::CompileOptions[IGNORE_CASE, ANCHORED]).inspect.should eq %(Regex.new("foo", Regex::Options[IGNORE_CASE, ANCHORED])) + end + + it "escapes" do + Regex.new(%("), :anchored).inspect.should eq %(Regex.new("\\"", Regex::Options::ANCHORED)) + end end end diff --git a/src/regex.cr b/src/regex.cr index 2b593397528b..23cc2b53406a 100644 --- a/src/regex.cr +++ b/src/regex.cr @@ -590,14 +590,32 @@ class Regex nil end + # Prints to *io* an unambiguous string representation of this regular expression object. + # + # Uses `#inspect_literal` if the literal representation with basic option flags + # is sufficient (i.e. no other options than `IGNORE_CASE`, `MULTILINE`, or `EXTENDED`). + # Otherwise the result is equivalent to `#inspect_extensive`. + def inspect(io : IO) : Nil + if (options & ~CompileOptions[IGNORE_CASE, MULTILINE, EXTENDED]).none? + inspect_literal(io) + else + inspect_extensive(io) + end + end + # Convert to `String` in literal format. Returns the source as a `String` in - # Regex literal format, delimited in forward slashes (`/`), with any - # optional flags included. + # Regex literal format, delimited in forward slashes (`/`), with option flags + # included. + # + # Only `IGNORE_CASE`, `MULTILINE`, and `EXTENDED` options can be represented. + # Any other option value is ignored. Use `#inspect` instead for an unambiguous + # and correct representation. # # ``` - # /ab+c/ix.inspect # => "/ab+c/ix" + # /ab+c/ix.inspect_literal # => "/ab+c/ix" + # Regex.new("ab+c", :anchored).inspect_literal # => "/ab+c/" # ``` - def inspect(io : IO) : Nil + def inspect_literal(io : IO) : Nil io << '/' Regex.append_source(source, io) io << '/' @@ -606,6 +624,22 @@ class Regex io << 'x' if options.extended? end + # Prints to *io* an extensive string representation of this regular expression object. + # The result is unambiguous and mirrors a Crystal expression to recreate an equivalent + # instance. + # + # ``` + # /ab+c/ix.inspect_literal # => Regex.new("ab+c", Regex::Options[IGNORE_CASE, EXTENDED]) + # Regex.new("ab+c", :anchored).inspect_literal # => Regex.new("ab+c", Regex::Options::ANCHORED) + # ``` + def inspect_extensive(io : IO) : Nil + io << "Regex.new(" + source.inspect(io) + io << ", " + options.inspect(io) + io << ")" + end + # Match at character index. Matches a regular expression against `String` # *str*. Starts at the character index given by *pos* if given, otherwise at # the start of *str*. Returns a `Regex::MatchData` if *str* matched, otherwise From aceacea2e7fa92911331f7edec5dd0d9b4064898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Mon, 13 May 2024 18:57:44 +0200 Subject: [PATCH 2/2] Make helper methods private --- src/regex.cr | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/regex.cr b/src/regex.cr index 23cc2b53406a..69dd500226a9 100644 --- a/src/regex.cr +++ b/src/regex.cr @@ -592,9 +592,13 @@ class Regex # Prints to *io* an unambiguous string representation of this regular expression object. # - # Uses `#inspect_literal` if the literal representation with basic option flags - # is sufficient (i.e. no other options than `IGNORE_CASE`, `MULTILINE`, or `EXTENDED`). - # Otherwise the result is equivalent to `#inspect_extensive`. + # Uses the regex literal syntax with basic option flags if sufficient (i.e. no + # other options than `IGNORE_CASE`, `MULTILINE`, or `EXTENDED` are set). + # Otherwise the syntax presents a `Regex.new` call. + # ``` + # /ab+c/ix.inspect # => "/ab+c/ix" + # Regex.new("ab+c", :anchored).inspect # => Regex.new("ab+c", Regex::Options::ANCHORED) + # ``` def inspect(io : IO) : Nil if (options & ~CompileOptions[IGNORE_CASE, MULTILINE, EXTENDED]).none? inspect_literal(io) @@ -615,7 +619,7 @@ class Regex # /ab+c/ix.inspect_literal # => "/ab+c/ix" # Regex.new("ab+c", :anchored).inspect_literal # => "/ab+c/" # ``` - def inspect_literal(io : IO) : Nil + private def inspect_literal(io : IO) : Nil io << '/' Regex.append_source(source, io) io << '/' @@ -632,7 +636,7 @@ class Regex # /ab+c/ix.inspect_literal # => Regex.new("ab+c", Regex::Options[IGNORE_CASE, EXTENDED]) # Regex.new("ab+c", :anchored).inspect_literal # => Regex.new("ab+c", Regex::Options::ANCHORED) # ``` - def inspect_extensive(io : IO) : Nil + private def inspect_extensive(io : IO) : Nil io << "Regex.new(" source.inspect(io) io << ", "