remove unnecessary captures within pipe expressions (fixes #78):

[dustypomerleau]

#78 is correct - keywords following pipes are not being scoped appropriately. They are being caught up in the 2nd capture group of "Pipe operator call without parenthesis." What's interesting is that the entire block seems redundant:

• The |> operator is captured separately elsewhere in the grammar (as keyword.operator.other.elixir), so removing the first capture does not affect scoping/highlighting of the pipe itself.
• If the first entity after the pipe is in fact a function call, then it will also be scoped appropriately (as entity.name.function-call.elixir) without any special capturing, so the 2nd capture group can be removed as well.
• If the first entity after the pipe is not a function call, then removing this block allows it to be captured by the appropriate pattern (such as keyword.control.elixir). Since this can be done without any explicit includes, this seems like all upside to me. We aren't deleting any parent scope for the entire expression, because only the sub-captures were used.

[dustypomerleau]

Ah, my apologies, it's slightly more complicated than that. The original author was trying to scope a situation where you pipe into a function, and also leave the parentheses off the function to the right of the pipe.

For example, instead of

arg |> my_func()

you write

arg |> my_func

When there are no additional arguments, mix format will not add parens here, so it's possible that some people prefer that. It does occasionally come up. I found this example in the main Elixir repo, but it is the rare exception rather than the rule.

It's easy enough to add another optional capture group for keyword.control.elixir between the 2 existing capture groups, but this gets messy very quickly. If the keyword is a guard clause/if statement, then it might not be immediately followed by a function call.

You might see something like:

arg |> if my_var > 3 do ...

In that case, our overly simplistic model breaks down, and my_var gets inappropriately scoped as entity.name.function-call.elixir (which is simply a shift of the exact problem we are trying to fix for keywords). My overall feeling is that it's better to remove the block entirely and let the other scopes shine through. The caveat being that you will need to add parens to the post-pipe function in order for it to be scoped appropriately. Since mix format prefers the parens when another argument is present, requiring them when no additional arguments are present seems consistent, rather than onerous (to me).

If the goal is for the extension's syntax highlighting to be correct for all valid Elixir syntax, irrespective of formatting convention, then I can go back to the drawing board—but I fear this could be a rather deep rabbit hole, for what was otherwise a simple request to get keywords back.

[axelson]

If the goal is for the extension's syntax highlighting to be correct for all valid Elixir syntax, irrespective of formatting convention, then I can go back to the drawing board—but I fear this could be a rather deep rabbit hole, for what was otherwise a simple request to get keywords back.

I would say that isn't a complete goal, especially since it is relatively infeasible with regex-based textmate grammars.

I think not highlighting arg |> my_func as a function call is a worthwhile tradeoff to keep the grammar relatively simple. I'll leave the PR open for a few days and do some testing locally as well as give others a chance to comment if they fell strongly, but then I'll go ahead and merge it.

[axelson]

Thanks for this ❤️