Complete in strings and comments
lalomartins opened this issue · 29 comments
I'd like to be able to write snippets for strings and comments. For example, language-coffee-script has a snippet for "#{interpolation}" which doesn't work so it had to hack around it somehow (I'm not quite sure how, couldn't find it in the source). Some snippets, such as lorem and legal, are clearly only useful (in source code files) inside a string or comment.
Currently it's not possible to complete in strings and comments, due to disableForSelector: '.comment, .string'. Furthermore, since autocomplete+ hijacks tab from snippets, I can't trigger the snippet by hand with tab either, if there's a symbol (fuzzy-)matching the snippet name.
I believe that does more harm than good; if the symbol provider works in strings and comments, why not snippets?
Even if it turns out people really don't like them, I'd suggest an alternative implementation is to match only snippets that actually have .string or .comment in the selector. Hacking around the devtools console, I managed to make that happen by passing only the current scope as the scope option to config.get. I managed to make that happen with the following code:
getSuggestions: ({scopeDescriptor, prefix}) ->
return unless prefix?.length
isStringOrComment = scopeDescriptor.getScopeChain().match(/\s([^ ]*\.(string|comment)\b[^ ]*)$/)
if isStringOrComment
# could also use isStringOrComment[1]
scopes = scopeDescriptor.getScopesArray()
scopeDescriptor = [scopes[scopes.length - 1]]
scopeSnippets = atom.config.get('snippets', {scope: scopeDescriptor})
@findSuggestionsForPrefix(scopeSnippets, prefix)(Not very happy with using a regex, but I couldn't find an API to check for a selector match; then again personally I'd rather not have this hack at all, just enable the provider and leave it at that.)
Your expectations of the package and the presence of snippets in comments is reasonable. I agree that filtering the snippets to the current scope when inside comments is the way to go.
@lalomartins would you be open to creating a PR for this?
I have it in a branch, so yes. I just need to check if there's a bug in it wrt punctuation (as punctuation would be the last scope in this case, which prevents the snippet from triggering if it's just before the closing quote).
Thanks for the quick feedback!
On the other hand, wouldn't it be better, both in terms of UX and code maintainability, to just remove the disableForSelector property and see if anyone complains? It's one less hack, and the behaviour would be less surprising.
remove the disableForSelector property
Yes, but I'd like to ensure we're only then suggesting snippets that are valid in comments. So it may somewhat invert the code you have and the way it goes about filtering the suggestions and having them displayed.
The thing is… without getting too philosophical… how do we know what's valid? I thought at first symbol completion in strings was useless, until I had to write the name of a class in an error message. If I'm writing a Python docstring, I might want a full if-else block as part of my code example. I'm sure you can think of other cases.
I just don't see the justification for “protecting” the user like that. It's not like autocomplete is forcing anything on you anyway, right? You don't want the snippet, just don't select it.
It got out there because people complained (rather vehemently) that
suggestions in comments were not desirable. Perhaps the solution is just to
make it configurable.
On Wed, Jun 24, 2015 at 1:04 PM Lalo Martins notifications@github.com
wrote:
The thing is… without getting too philosophical… how do we know what's
valid? I thought at first symbol completion in strings was useless, until I
had to write the name of a class in an error message. If I'm writing a
Python docstring, I might want a full if-else block as part of my code
example. I'm sure you can think of other cases.I just don't see the justification for “protecting” the user like that.
It's not like autocomplete is forcing anything on you anyway, right? You
don't want the snippet, just don't select it.—
Reply to this email directly or view it on GitHub
#55 (comment)
.Best Regards,
Joe Fitzgerald
Director, Pivotal Cloud Foundry Solutions
Mobile: (303) 888-3964 | jfitzgerald@pivotal.io
LinkedIn: @jfitzgerald | Twitter: @joefitzgerald | Github: @joefitzgerald
I can relate to that point of view, but then the solution is to disable autocomplete-plus in comments/strings, not autocomplete-snippets. Having a suggestions box with everything but snippets is not only too surprising in terms of UX, it also doesn't solve the problem of suggestions not being desirable 😸
And by the way, disabling the autocomplete box in comments/strings (with or without config option) would also solve my own problem, as I'd be able to expand snippets by pressing tab. So +1 for that.
You can do this today in ac+ preferences: scope blacklist.
In which case it seems to me the right fix is to remove the disableForSelector from autocomplete-snippets entirely, and then change ac+'s default scope blacklist to remove it from comments and strings
The default for ac+ won't change because it would prevent precisely the kind of use case you described: suggestions in comments.
But isn't that exactly what people complained about?
You can't have it both ways, you know. If suggestions in comments are disruptive, you want them all off. If you want suggestions in comments, it makes no sense to disable only snippets, since you can simply ignore them when the list does pop up. There is no benefit to blacklisting only snippets.
@lalomartins you are quite right - you can't have it both ways: defaults can never please 100% of people. Thats why it's configurable in ac+. Perhaps the fix is just to make it configurable in autocomplete-snippets too. There is benefit to blacklisting only snippets, for some people. Just not you :)
I honestly believe you're mistaken on that point; as I said, if the menu is coming up anyway, you can just ignore the items you're not interested in. If there is a benefit, it's too marginal to increase maintenance costs with a config option.
But it doesn't matter what I believe, as long as I can have mine back without having to keep a fork for just that reason :-)
@lalomartins As I said, it's impossible to match everyone's desires with defaults. My statement doesn't belittle or disregard your strongly held belief that a default should be a certain way. That's why we make some things configurable. If we were to disable ac+ in comments entirely it would suppress a whole category of providers (and other things we cannot anticipate) like a JavaDoc or a JSDoc provider.
As I said prior, changes in the autocomplete-snippets disableForSelector and snippet filtering logic seem like a prudent thing, so that snippets are displayed in comments when appropriate. I would gladly accept a PR that achieves this!
I'm sorry if it sounded like I was belittling anything. I'm just trying to make a point that doesn't seem to have made it across: that if the menu pops up, it really makes no difference to have more options in it or less.
Meanwhile: I'll make a PR. To be clear, do you want exactly the behaviour I described in the OP? Filter so you only get snippets that explicitly match comments/strings, with no extra configuration?
No I was just concerned you thought I wasn't listening to you. I am, and I fully respect your desire for the functionality you described.
The thought was that many of the default providers would also suppress in comments, preventing suggestions from occurring. But your point is noted: SymbolProvider does not suppress in comments (intentionally) meaning the suggestion list will also display. I'll talk with @benogle about this.
Yes. If there's a consensus that various other providers will suppress, or make it configurable, I'd be behind that, might even be willing to pitch in some code.
Should also think about how that will go if it's many providers — configure in ac+, or in each provider? Provide an API to make it easier? Ensure some config UI consistency?
There was another short discussion on this at atom/autocomplete-plus#377, which was why the disableForSelector attr was added to this, ac-css, and ac-html.
I'm just trying to make a point that doesn't seem to have made it across: that if the menu pops up, it really makes no difference to have more options in it or less.
I disagree with this. 1 super relevant suggestion is much better than 1 relevant suggestion and 4 garbage suggestions.
changes in the autocomplete-snippets disableForSelector and snippet filtering logic seem like a prudent thing, so that snippets are displayed in comments when appropriate.
I both like and dislike this idea:
- It would be nice when in comments / strings to only display the snippets that apply to comments / strings. Snippets are often scoped way too liberally, so having this sort of check in place would be nice.
- I dislike it because it is a special case, and you can achieve the same thing with configuration in the package's snippet cson file by nulling out snippets in comments / strings like: https://github.com/atom/language-html/blob/master/snippets/language-html.cson#L373
We could make snippet display strict-match scopes or something. But that is disingenuous as the snippets can be expanded at .text.html .whatever when they are defined as .text.html.
Not sure what the best thing to do is.
I disagree with this. 1 super relevant suggestion is much better than 1 relevant suggestion and 4 garbage suggestions.
I'd agree if that was what happens, but how is a bunch of symbols (and it's never just one) more relevant inside a string than snippets written specifically for that purpose? Or, for that matter, any snippets, if we think of Python docstrings, for example.
I think in the end — for a portion of people/situations, all completions are useful in strings and comments, for others, none. The “some” portion I'm willing to bet is pretty small, and I doubt most of those would agree symbols are more relevant than snippets.
So, this is probably something best left to the user — and there's already a mechanism for that with the scope blacklist (and, as you say, you can null out snippets if they're bothering you). So I think the simplest solution is to just remove the disableForSelector and have no confusing special cases.
This important. Autocomplete plus is amazing, but with option it renders LaTeX autocompletion basically useless, since it's not problem to complete snippets in your text, but when in math-mode (which apparently gets detected as string) there is no autocompletion anymore and it's a little annoying to recompile the whole app just for this one option. It would be great if this gets changed fast..
It seems that snippets in attribute are working, one just needs to expand them with tab instead of enter.
Hi, sorry for wading into what seems to be a long dead channel, but it seems to me that some snippets are still disabled inside, for instance, quotes. Is that not the case?
What kind of quotes do you mean?
It seems that within "" and '' the autocomplete doesn't load snippet suggestions.
@ryantimjohn it doesn't show the list of sugestions, but when you actually expand them (even within ""), it works, at least for me. You have to use TAB key though:
