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 a problem with the do Elixir keyword #61

Closed
wants to merge 1 commit into from
Closed

Fix a problem with the do Elixir keyword #61

wants to merge 1 commit into from

Conversation

whatyouhide
Copy link
Contributor

As of my last comment in #60, the do Elixir keyword didn't work with stuff like:

defmodule M do
  # no `end` here

Now it does; I tested this behavior also with def func do...end, case [...] do...end and some other constructs, everything seems to work and, more importantly, nothing seems to be broken.

Before this commit, this use of the `do` keyword in Elixir would be
autocompleted with an `end` by vim-endwise:

  do
    # stuff
  end

But this would not:

  defmodule A do
    # no `end` here

By replacing a whitespace match (\s) with a wildcard (*) in the Elixir regex,
this now works.
@@ -19,7 +19,7 @@ augroup endwise " {{{1
autocmd FileType elixir
\ let b:endwise_addition = 'end' |
\ let b:endwise_words = 'case,cond,bc,lc,inlist,inbits,if,unless,try,receive,function,fn,do' |
\ let b:endwise_pattern = '^\s*\zs\%(case\|cond\|bc\|lc\|inlist\|inbits\|if\|unless\|try\|receive\|function\|fn\|do\)\>\%(.*[^:]\<end\>\)\@!' |
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, I just replaced the whitespace match \s with the . wildcard.

@tpope
Copy link
Owner

tpope commented Jul 15, 2014

See Ruby for an example of how to do this properly. I don't know Elixir but I'm guessing loosening the regex for all keywords will have unintended consequences.

I already reverted the previous attempt so build your submission on the latest so it will merge properly.

@tpope
Copy link
Owner

tpope commented Jul 15, 2014

Also consider if this enables other keywords to be removed. I see if always takes a do, for example.

@whatyouhide
Copy link
Contributor Author

Yep, I think this needs a little revision. I'll try to do it when I'll have some spare time. Thanks for now!

@rbishop
Copy link

rbishop commented Aug 4, 2014

I took a stab at this in another PR, but I think I've over thought it at first glance. Elixir actually has a much more consistent syntax than Ruby. All "blocks" in Elixir use the do word. So I think any line ending in do should complete with an end. The only exception are one-liner function definitions and one-liner if/do/else, which will have a do: (note the colon).

Could Elixir's end wise be as simple as auto-completing the end for any line ending in do and ignoring lines that end in do:?

@whatyouhide
Copy link
Contributor Author

@rbishop not quite so. Here you can see keywords like receive/after aren't followed by a do.

@tpope
Copy link
Owner

tpope commented Aug 5, 2014

Cited example has a receive do, and after does not have an end.

I think the proposed logic is solid. You might look at the sh support for inspiration.

@rbishop
Copy link

rbishop commented Aug 5, 2014

receive takes a do, but after does not. However, I can't think of a case where you will have an after outside of a receive.

Thanks @tpope. I'll have a look.

@whatyouhide
Copy link
Contributor Author

I'm so sorry, yesterday I was in a hurry and read the Erlang example, not the Elixir ones 😭. Yes then, ends after every do should be fine.

@gwww
Copy link

gwww commented Sep 12, 2014

My first time hacking a plugin, please be forgiving. Based on the discussion above by @rbishop this works for me. Not sure it is worthy of a merge as I only know enough Elixir to be dangerous.

autocmd FileType elixir
      \ let b:endwise_addition = 'end' |
      \ let b:endwise_words = 'do\s*$' |
      \ let b:endwise_syngroups = 'elixirKeyword'

tpope added a commit that referenced this pull request Sep 13, 2014
Closes #65.

References #61.
@tpope
Copy link
Owner

tpope commented Sep 13, 2014

@gwww closer would be

autocmd FileType elixir
      \ let b:endwise_addition = 'end' |
      \ let b:endwise_words = 'do' |
      \ let b:endwise_pattern = '\<do\ze\s*$' |
      \ let b:endwise_syngroups = 'elixirKeyword'

Try that a few days and if it works send a pull request.

@tpope tpope mentioned this pull request Sep 13, 2014
@rbishop
Copy link

rbishop commented Sep 14, 2014

I'm not sure if this matters at all, but I noticed elixir-lang/vim-elixir has a lot of keywords defined: https://github.com/elixir-lang/vim-elixir/blob/7a0a1688601da4ae48e862e5840c832f04e63364/syntax/elixir.vim#L106-L120

Would this mean that endwise_syngroups would have to have most of them (or all of them?) listed?

@tpope
Copy link
Owner

tpope commented Sep 14, 2014

Not if we're matching on do.

@rbishop
Copy link

rbishop commented Sep 14, 2014

Great point. I'll start using this and see if anything surprising happens. Thank you @tpope and @gwww

phongvcao pushed a commit to phongvcao/vim-endwise that referenced this pull request May 24, 2015
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.

4 participants