elixir-editors / language-elixir

Elixir language support for the Atom editor.
Other
181 stars 40 forks source link

Remove `\b` and anchor string ("documentation") from doc snippets #68

Closed ymtszw closed 8 years ago

ymtszw commented 8 years ago

Currently, snippet body for @doc is defined as:

'body': '@doc """\n\b\b${0:documentation}\n"""'

Same goes for @moduledoc and @typedoc.

My understanding of reason for use of \b (in this case backspace) is, to negate additional indentation inserted by editor. Since without \bs, generated snippet will become like this:

2016-06-10 11 11 04

Obviously we want this:

@doc """
documentation
"""

With \bs, it seems this desired behavior achieved. But I found today that use of \b produces somewhat strange result which can be noticed in git diffs:

+  @doc """
+Some documents.
+  """

Whitespaces for indentation cannot be seen in diff, but they seem to exist in Atom editor. And when you try to commit, in the summary of staged contents it appears:

+  @doc """
+  ^H^HSome documents.
+  """

Looks like on snippet generation, \b is not actually just deleting leading whitespaces (indent), but rather inserted as real \b characters. I can confirm this by regex-searching in editor:

backspace

Contaminated \b is not so pleasant (at least to me).

After some trial I found an OK solution: simply remove \bs and anchor string "documentation":

 'body': '@doc """\n$0\n"""'

Produced result should be like this (trailing whitespace highlighting is from my setting):

2016-06-10 11 27 55

The cursor is placed right after (highlighted) indentation, so we can start typing at once. This should work at any indent level, for either soft or hard tabs. Since "documentation" is trivial description in this case, we can safely omit that.

If it's acceptable I can open PR.

keathley commented 8 years ago

Its fine if you want to open a PR for this. Leaving the backspace character is definitely not the desired behaviour. The fix you're describing is the original implementation of the doc snippet. The original issue was that even though the cursor was positioned correctly, because the previous whitespace was selected when the user started typing the text would replace the whitespace. The end result was that the documentation text wasn't indented to the right level. If you can find a snippet that indents correctly and doesn't leave the backspace character in there then that would be great.

ymtszw commented 8 years ago

@keathley Actually, I found that flaw (previous whitespace was selected) after I opened this issue :P We cannot start typing right after the snippet expansion with the above fix. I'll look for more precise solution for a while.

Thanks for reply!

keathley commented 8 years ago

No worries. This has been an issue for a while. If you can come up with something I'd be happy to see it.

ymtszw commented 8 years ago

Hi, it's been a while but I couldn't find perfect solution. Though I've found another clunky behavior related to \b insertion. Might be already on your radar.

Cursor position after snippet expansion is indeed correct, but when you delete placeholder string (documentation) right after expansion, cursor disappears.

2016-07-04 0 29 37

(The cursor is actually at the position marked by red arrow, but not visible)

And it only comes back when you actually type something, or move up/down the cursor (it stays disappeared when you move it left). This maybe only occurring in my environment, would be nice to hear about this from someone else.

Personally I'm inclined to my proposed change for now (i.e. revert 0eaaec1 back to original impl). Although it isn't perfect, but at least 'cleaner'.

Want to hear your thoughts!

keathley commented 8 years ago

I agree. We should revert this to the original implementation. Hopefully atom will either fix this or provide a solution in the future.

ymtszw commented 8 years ago

Thanks, opened PR.

keathley commented 8 years ago

Closing this now that #73 has been merged.