Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #707 (partly) #1057

Closed
wants to merge 1 commit into from
Closed

Fix #707 (partly) #1057

wants to merge 1 commit into from

Conversation

andreyorst
Copy link
Contributor

Hi, I've partly fixed #707 issue for Elixir's do: blocks. However there are still some problems, which are also exist in current implementation of def ... do ... end blocks.

With my changes the example from the linked issue no works:

defmodule ElixirExampleOne do
  def func_one do
    IO.puts("hi 1")
  end

  def func_two, do: IO.puts("hi 2")
end

defmodule is correctly matched with the last end. The def func_two, do: IO.puts("hi 2") is completely skipped.

However this will not work:

defmodule ElixirExampleOne do
  def func_one do
    IO.puts("hi 1")
  end

  def func_two, 
    do: IO.puts("hi 2")
end

Now, I must note, that even without my patch, the following construct will not work either:

defmodule ElixirExampleOne do
  def func_one
  do
    IO.puts("hi 1")
  end
end

This is due to the fact, that the check if line based, and we simply check if do is on the same line, which is not strict enough. I'm not sure how one would do a better check, perhaps with a search, but this does not guarantee that we'll find correct keyword, and not something that also can match.

Given that, this PR fixes specifically the def func_two, do: IO.puts("hi 2") case.

@andreyorst andreyorst changed the title Fix #707 Fix #707 (partly) Dec 6, 2020
@andreyorst
Copy link
Contributor Author

I've tried a bit to do a traversal search that checks for possible definitions when matching from the end, e.g. like here

def foo(x, y)
  do
  when x, do: y
  if x
    do
    y
  end
end

And when traversing from the front I think it is possible to get reliable results as we're only searching forward for the end key, but when searching backwards we have to dynamically check what keyword we might need, as end can match with a lot of stuff. Like here, end could have matched with if, then with when then with do and only then with foo, and I can't easilly come up with some heuristics to determine the invariant here. Not suggesing that this is impossible, but I don't know if I can do that reliably enough.

Which actually gets me thinking if Elixir could just match against only do end, and ignore the rest, however this would make impossible to use all the features Smartparens provide like slurping an barfing, so this is not an option.

@andreyorst
Copy link
Contributor Author

I'm not sure if this can be closed in favor of #1058 or not, as #1058 may not be very optimal, so a solution should instead be built upon this approach. Feel free to decide which implementation suits the project better

@andreyorst
Copy link
Contributor Author

Now that I've worked out most of the quirks in #1058, I think this can be closed.

@andreyorst andreyorst closed this Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do-block syntax in Elixir confuses show-smartparens-mode
1 participant