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

Improve / disable smart import mapping #568

Closed
blueyed opened this issue May 11, 2016 · 23 comments
Closed

Improve / disable smart import mapping #568

blueyed opened this issue May 11, 2016 · 23 comments

Comments

@blueyed
Copy link
Collaborator

blueyed commented May 11, 2016

Given (at least) #524, #559 and #567 we should consider disabling this by default and/or improving it (also with regard to finding it).

You can quite easily find it by searching for "import" in :h jedi-vim though.

So, calling out to the reporters (@hjwp @tinruufu @chkur) of the issues mentioned above:
What do you dislike about it? (it it too slow? are you keeping to type ahead?)

I could imagine to skip/abort the process in case the user keeps typing and/or only trigger it after e.g. 50ms after the space has been typed.
What do you think?

I've got used to it myself, but can understand that it's surprising - but would still not say to disable it by default (but without knowing how many like this feature).
Let's have a vote in the next comment.

/cc @aliev as the original inventor of this.. :) (in #454).

@blueyed
Copy link
Collaborator Author

blueyed commented May 11, 2016

Please vote up if you like the feature in general, down if not.

@blueyed
Copy link
Collaborator Author

blueyed commented May 11, 2016

Do you have it enabled currently?

@blueyed
Copy link
Collaborator Author

blueyed commented May 11, 2016

Would you like to have it triggered after a configurable timeout of e.g. 50ms by default, so that it would get skipped when typing the i directly after the space?

@blueyed
Copy link
Collaborator Author

blueyed commented May 11, 2016

I think it will be easier to implement using a callback in Neovim (since we're using a hack via 'updatetime' for g:jedi#show_call_signatures_delay already), so here's another question for up/down voting:
Are you using Neovim?

@blueyed
Copy link
Collaborator Author

blueyed commented May 11, 2016

Is having the same timeout as with g:jedi#show_call_signatures_delay OK (which is 500ms) by default?
This setting could be renamed then and be applied for both things.

@hjwp
Copy link

hjwp commented May 13, 2016

Hi there, to answer the first question, the reason I wanted to switch it off is I couldnt get used to it -- I did try (not very hard admittedly) to train myself, but I always found myself typing the "import" keyword, and finding it appear twice.

@hjwp
Copy link

hjwp commented May 13, 2016

regarding the idea of a short delay before the completion starts. As someone who doesn't want the feature, as a default behaviour i think that would solve my problem, but: wouldn't that make the experience worse for the people who do like the feature?

In that respect, maybe it's unlike call signatures? Because that's something I sometimes want to see and sometimes don't want, so I don't mind waiting a bit if, as I'm coding, just now is one of the times i do want to see it. but the import keyword autocompletion, i think (maybe?) that's something you either want or don't want, forever.

@hjwp
Copy link

hjwp commented May 13, 2016

In any case, I'm very happy now that I've got the feature switched off. My docs suggestion for improvement would be: rename that section in the docs? and/or the name of the feature? to me, smart_auto_mappings doesn't immediately bring to mind "ah yes, this is the feature that autocompletes the word "import""

@davidhalter
Copy link
Owner

Thanks for bringing this up! I'm actually also undecided if it's a great idea. I've gotten used to it, but it still has that kind of strange feeling where when you write import after it, it's just a duplicate. Of course you can train to not use it, but it's just not the same behavior that a normal IDE has.

@tinruufu
Copy link

i'm okay with the feature, i just think it's a bad default and not in keeping with the rest of the stuff this project does for me

@blueyed
Copy link
Collaborator Author

blueyed commented Jul 22, 2016

For reference: it's also problematic with iabbr: #495 (comment): if <space> is mapped it seems to not expand abbreviations anymore.
Not sure if we could somehow detect and skip/simulate it.

@davidhalter
Copy link
Owner

Well, I would also be fine with another default. That would probably cause less issues.

I have realized that smart_auto_mappings will not contain as much stuff as I would have liked to. There's only very few examples where auto completion allows you to add a keyword just right after a space (and most of them are difficult to handle without a lot of gain). The thing that would be really cool is adding () to function calls, but that won't work either, because you also need to ignore the closing ) and opening ( if the user types it. That's just a nightmare to do in VIM.

@tinruufu
Copy link

also, just because something seems callable doesn't mean you want to call it; my least favourite thing about ipython is that it dumps a ( at the end of tab-completed class names

@blueyed
Copy link
Collaborator Author

blueyed commented Jul 23, 2016

My current plan would be:

  1. Disable it by default, unless there is a better fix for it shortly.
  2. The better fix would be to detect if something else was typed by the time the autocompletion actually returns from being triggered, and then skip the smart insertion: so it would still trigger the autocompletion (which can take up to a few seconds), but at least then the typeahead buffer would be used. Still not really nice, but when combined with async completion (e.g. from deoplete) it might actually be really nice.

@davidhalter
Copy link
Owner

also, just because something seems callable doesn't mean you want to call it; my least favourite thing about ipython is that it dumps a ( at the end of tab-completed class names

I totally agree. That's why I ended up not adding a (.

My current plan would be: [..]

I agree. That's probably a more sane default. Feel free to change it.

@balta2ar
Copy link

balta2ar commented Aug 2, 2016

I am another person who was really surprised by this behavior. I'm slowly getting used to it and I see benefits, but that was completely unexpected. I have no strong feelings against it, it's just a matter of habit and time to change it. Maybe disabling by default is a good idea indeed.

@tallus
Copy link

tallus commented Jan 31, 2017

I think the proposed improvement should solve this as well, but as someone who likes the feature I wanted to make sure the issue of inserting import twice when copying and pasting (in insert mode) gets flagged separately from typing.

If I'm hit with this when I'm typing, its an annoyance, but the fact that I'm annoyed means I'm aware of it and can fix it. This is not the case when I'm copying and pasting.

  1. Pasting inserts text quick enough that the autocomplete popup doesn't appear (or disappears fast enough it might as well not) so there is no visual trigger to tell you what's happening
  2. It inserts in the middle of the inserted text, which makes it harder to spot (I'm Dyslexic, and this is exactly the kind of mental blind spot I have) and since you already know what the text you want is, you are less likely to be paying attention.

The behaviour with pasting. I think is in conflict with our mental models of how things work. My mental model for auto-complete, at least, goes something like this: 'When I type the computer looks at what I'm typing and tries to make helpful suggestions about what I want to type next'.

Copy and pasting behaviour is directly contrary to this in two ways;

  1. I'm not typing, I'm using the mouse.
  2. When this behaviour is trigged when pasting, it gets triggered when I'm pasting things in the form form x import y. That is to say, I've already told the computer exactly what I want so there is no room for auto-suggestion.

The upshot of this is it seems like unexpected, even illogical behaviour, and it easy to miss, so it can introduce real errors that won't be noticed until another time.

  • I had to consciously reason it out before I even realised what was causing it, it just seemed like something was doubling import for no reason.

@blueyed
Copy link
Collaborator Author

blueyed commented Jan 31, 2017

Well, normally 'paste' should be active/set when pasting.

We should just use a timer for this, and then it requires a recent Vim/Neovim.
Should not be too hard.

@Flimm
Copy link
Contributor

Flimm commented May 19, 2017

The issue for me is that it types import for me automatically, which I hate. Show me the pop-up, but don't mess with my expection that typing from x import y produces from x import y instead of from x import import y.

@blueyed
Copy link
Collaborator Author

blueyed commented May 19, 2017

See #708 for a fix.
Feedback would be appreciated as always.

@blueyed
Copy link
Collaborator Author

blueyed commented May 19, 2017

With something like deoplete/deoplete-jedi it is actually more useful to only add the import, but not trigger (jedi-vim's) omnicompletion, since deoplete-jedi will provide the completion already (which I like more in general).

blueyed added a commit to blueyed/jedi-vim that referenced this issue Jul 5, 2017
blueyed added a commit to blueyed/jedi-vim that referenced this issue Aug 7, 2017
blueyed added a commit to blueyed/jedi-vim that referenced this issue Apr 11, 2018
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jul 15, 2018
blueyed added a commit to blueyed/jedi-vim that referenced this issue Mar 28, 2019
blueyed added a commit to blueyed/jedi-vim that referenced this issue Mar 28, 2019
@davidhalter
Copy link
Owner

Thanks for your work here. I don't really have time to test this. Other things are more important. If you think this is good enough and if you have tested it well, please self-merge. I would of course prefer if another user here tests it :)

blueyed added a commit to blueyed/jedi-vim that referenced this issue Apr 5, 2019
davidhalter pushed a commit that referenced this issue Apr 5, 2019
@davidhalter
Copy link
Owner

Since it's now disabled by default (thanks for looking into it!), I'm closing this as solved.

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

No branches or pull requests

7 participants