Code.string_to_quoted reporting wrong column for start of a Map about elixir HOT 15 CLOSED

@doorgan I'll tackle that in a few minutes in the PR I have open

from elixir.

1.17.0 onwards.

from elixir.

What are you using this for? I believe this has been discussed before, but we cannot fix this without making structs somewhat inconsistent:

iex(3)> Code.string_to_quoted! "%Foo{foo: 321}", columns: true
{:%, [line: 1, column: 1],
 [
   {:__aliases__, [line: 1, column: 2], [:Foo]},
   {:%{}, [line: 1, column: 5], [foo: 321]}

In that case, we show the starting line as the one in {, but perhaps that's a smaller problem to live with. /cc @lukaszsamson

from elixir.

This is in Sourceror. When matching on a map literal and patching that node the column is off by 1 character so when you go to replace that range of characters with a replacement map it ends up with %%{a: 1}

from elixir.

But if this is a wontfix would you suggest that in Sourceror that for just map literals the column is adjusted by -1?

from elixir.

Pinging @doorgan as well then. :)

from elixir.

We can fix this in sourceror's range function itself if it's a known issue that won't be fixed soon!

from elixir.

I think the biggest issue is that, if we fix it for maps, you still should not offset the %{}inside struct. So I wonder which one is more or less work?

from elixir.

Isn’t it fixed already on master? See fd4e6b5

from elixir.

Thank you @lukaszsamson. I remember we had a recent discussion about this, but I didn't remember it was already fixed. :)

from elixir.

@lukaszsamson thank you :)

from elixir.

@josevalim if fixed on master how should Sourcer address this for the time being? I don't want to wait on the next Elixir release but I can do a patch in Sourcerer to address this temporarily and then I guess Sourceror should do a version limit up to the current minor version of Elixir then remove that patch for the next release?

from elixir.

I guess I could really just fix this in LiveView Native and limit on the Elixir version for now then release without the patch later

from elixir.

@bcardarella What I used to do was to backport fixes to main to the vendored Elixir modules in Sourceror. I stopped doing that when I raised the minimum elixir version requirement, but I can see us keep doing it 🤔 Or doing Version checks for known bugs

from elixir.

@josevalim I'm going to implement a fix in Sourceror using @doorgan's suggestion on branching based on version. Do you know yet what the likely version will have the fix @lukaszsamson mentioned? Next minor?

from elixir.