Hi!
Done: https://github.com/elixir-lang/elixir/pull/11995
Thx & have a great day!
Q
On Monday, July 18, 2022 at 5:40:41 PM UTC José Valim wrote:
> I agree with your points. A PR with your additional tests and specs will
> be very welcome! Thank you!
>
> On Mon, Jul 18, 2022 at 7:25 PM Quentin Crain <czr...@gmail.com> wrote:
>
>> Hi!
>>
>> I hope to not draw this out .. afaict the goal of the PR is to preserve
>> the method the developer used in providing options. So, I am a bit confused
>> by possibly doing any converting (presumably from options as atoms to their
>> binary char equivalents). This is what I had in mind though and what I
>> implemented locally.
>>
>> This seems to me to be so by the change in the type of opts field in the
>> structure to OR with a list/list(atom); with the change to how Regexs are
>> inspected being a 2nd evidence.
>>
>> I have added the following test which seems to show this:
>>
>> --- a/lib/elixir/test/elixir/inspect_test.exs
>> +++ b/lib/elixir/test/elixir/inspect_test.exs
>> @@ -864,6 +864,7 @@ test "regex" do
>> assert inspect(~r" \\/ ") == "~r/ \\\\\\/ /"
>> assert inspect(~r/hi/, syntax_colors: [regex: :red]) ==
>> "\e[31m~r/hi/\e[0m"
>>
>> + assert inspect(Regex.compile!("foo", "i")) == ~S'~r/foo/i'
>> assert inspect(Regex.compile!("foo", [:caseless])) ==
>> ~S'Regex.compile!("foo", [:caseless])'
>> end
>>
>> Finally, I also added some tests for Regex.opts which show options' type
>> are preserved:
>>
>> test "opts/1" do
>> assert Regex.opts(Regex.compile!("foo", "i")) == "i"
>> + assert Regex.opts(Regex.compile!("foo", [:caseless])) == [:caseless]
>> end
>>
>> Though, reviewing the type spec for opts, I believe it should be updated
>> to reflect that options are a binary OR atom list:
>>
>> - @spec opts(t) :: String.t()
>> + @spec opts(t) :: String.t() | [term]
>> def opts(%Regex{opts: opts}) do
>> opts
>> end
>>
>> I apologize for the length of this reply but it does seem the present
>> implementation is contra to my thoughts on preferring the binary
>> representation of options over a list.
>>
>> Respectfully,
>>
>> Q (Quentin)
>>
>> On Monday, July 18, 2022 at 7:04:51 AM UTC José Valim wrote:
>>
>>> The PR does not attempt to do a conversion back to known cases, so
>>> adding such feature is still welcome!
>>>
>>> On Mon, Jul 18, 2022 at 9:02 AM José Valim <jose....@dashbit.co> wrote:
>>>
>>>> Actually, a PR was already sent here:
>>>> https://github.com/elixir-lang/elixir/pull/11991 :)
>>>>
>>>>
>>>> On Mon, Jul 18, 2022 at 8:57 AM José Valim <jose....@dashbit.co> wrote:
>>>>
>>>>> Yes. And if any unknown option is given, perhaps we should store the
>>>>> underlying options in the struct and change the inspect representation to
>>>>> output Regex.compile!(…)
>>>>>
>>>>> Please open up an issue and PRs welcome too!
>>>>>
>>>>> On Mon, Jul 18, 2022 at 05:41 Quentin Crain <czr...@gmail.com> wrote:
>>>>>
>>>>>> Hi!
>>>>>>
>>>>>> I hope I am acting appropriately here!
>>>>>>
>>>>>> Per this elixirforum post:
>>>>>> https://elixirforum.com/t/is-it-normal-that-regex-compile-2-does-not-print-regex-modifiers-when-atoms-are-passed-in-options/48992
>>>>>>
>>>>>> When atoms are used as options to Regex.compile, they are not
>>>>>> translated into their character equivalents and added to the Regex
>>>>>> struct:
>>>>>>
>>>>>> iex(6)> Regex.compile("foo", "i")
>>>>>> {:ok, ~r/foo/i}
>>>>>> iex(7)> Regex.compile("foo", [:caseless])
>>>>>> {:ok, ~r/foo/}
>>>>>>
>>>>>> Should they be? I have an initial implementation waiting if so . . .
>>>>>>
>>>>>> Respectfully,
>>>>>>
>>>>>> Quentin (Q)
>>>>>>
>>>>>> --
>>>>>> You received this message because you are subscribed to the Google
>>>>>> Groups "elixir-lang-core" group.
>>>>>> To unsubscribe from this group and stop receiving emails from it,
>>>>>> send an email to elixir-lang-co...@googlegroups.com.
>>>>>> To view this discussion on the web visit
>>>>>> https://groups.google.com/d/msgid/elixir-lang-core/30477ff6-7eca-4e66-a118-2bf3920abd34n%40googlegroups.com
>>>>>>
>>>>>> <https://groups.google.com/d/msgid/elixir-lang-core/30477ff6-7eca-4e66-a118-2bf3920abd34n%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>>> .
>>>>>>
>>>>> --
>> You received this message because you are subscribed to the Google Groups
>> "elixir-lang-core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to elixir-lang-co...@googlegroups.com.
>>
> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/elixir-lang-core/5ff85784-a306-4c8c-879a-e55d62eb4d91n%40googlegroups.com
>>
>> <https://groups.google.com/d/msgid/elixir-lang-core/5ff85784-a306-4c8c-879a-e55d62eb4d91n%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>>
>
--
You received this message because you are subscribed to the Google Groups
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/elixir-lang-core/806dc300-ae0f-4a03-85f9-d29c487a6191n%40googlegroups.com.
