Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Complete in strings and comments #55

Open
lalomartins opened this issue Jun 24, 2015 · 29 comments
Open

Complete in strings and comments #55

lalomartins opened this issue Jun 24, 2015 · 29 comments

Comments

@lalomartins
Copy link

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.)

@joefitzgerald
Copy link
Contributor

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.

@joefitzgerald
Copy link
Contributor

@lalomartins would you be open to creating a PR for this?

@lalomartins
Copy link
Author

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!

@lalomartins
Copy link
Author

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.

@joefitzgerald
Copy link
Contributor

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.

@lalomartins
Copy link
Author

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.

@joefitzgerald
Copy link
Contributor

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

@lalomartins
Copy link
Author

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.

@joefitzgerald
Copy link
Contributor

You can do this today in ac+ preferences: scope blacklist.

@lalomartins
Copy link
Author

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

@joefitzgerald
Copy link
Contributor

The default for ac+ won't change because it would prevent precisely the kind of use case you described: suggestions in comments.

@lalomartins
Copy link
Author

But isn't that exactly what people complained about?

@lalomartins
Copy link
Author

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.

@joefitzgerald
Copy link
Contributor

@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 :)

@lalomartins
Copy link
Author

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 :-)

@joefitzgerald
Copy link
Contributor

@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!

@lalomartins
Copy link
Author

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?

@joefitzgerald
Copy link
Contributor

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.

@lalomartins
Copy link
Author

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?

@benogle
Copy link
Contributor

benogle commented Jun 30, 2015

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.

@lalomartins
Copy link
Author

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.

@lalomartins
Copy link
Author

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.

@dbischof90
Copy link

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..

@petrchutny
Copy link

petrchutny commented Mar 10, 2017

It seems that snippets in attribute are working, one just needs to expand them with tab instead of enter.

@ryantimjohn
Copy link

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?

@petrchutny
Copy link

What kind of quotes do you mean?

@ryantimjohn
Copy link

It seems that within "" and '' the autocomplete doesn't load snippet suggestions.

@petrchutny
Copy link

@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:
2017-12-30 12 43 28

@ryantimjohn
Copy link

ryantimjohn commented Dec 30, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants