The Wayback Machine - https://web.archive.org/web/20221207070022/https://github.com/atom/autocomplete-plus/pull/612
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

bugfix: auto indentation after suggestion confirm #612

Merged
merged 1 commit into from Jan 19, 2016
Merged

bugfix: auto indentation after suggestion confirm #612

merged 1 commit into from Jan 19, 2016

Conversation

fruggiero
Copy link
Contributor

@fruggiero fruggiero commented Nov 14, 2015

Added an instruction to trigger the auto indentation after a suggestion is confirmed.

(It is a bugfix for this issue)

@fruggiero fruggiero changed the title fixed auto indentation after suggestion confirm bugfix: auto indentation after suggestion confirm Nov 16, 2015
@benogle
Copy link
Contributor

benogle commented Nov 17, 2015

I wonder if this will be problematic in other cases. You may not always want it to auto indent. e.g. you writing coffee

someFunction: ->

someOther| # completes `someOtherFunction`

In this case, it would autoindent to be a child of the function which could be really really annoying.

@jeancroy
Copy link
Contributor

jeancroy commented Nov 17, 2015

You may not always want it to auto indent.

Looking at the gif, the intention was to make auto completing react the same as actually typing the characters. That's probably always a good behavior.

However the fix looks a bit like a band-aid, or a minimum change solution.

It looks like the underlying issue is that not everything that listen for a key press get notified after autocomplete. The one apparent here is the logic that deal with increaseIndentPattern and decreaseIndentPattern

@fruggiero
Copy link
Contributor Author

fruggiero commented Nov 18, 2015

I stepped through the code and noticed there are some difference within calling editor.insertText (the one that atom calls when you manually type a char) vs calling directly selection.insertText (the one that autocomplete-plus use on suggestion confirm).

The editor.insertText function create an hash of options and pass it to the selection.insertText function.
In particulary some options that deal with auto indentation.

By calling selection.insertText directly without options we miss the auto indentation functionality.

My last commit fix this, by passing the same indentation options that atom use.

Now the behaviour is exactly the same as when you manually type characters.
So no problem for this @benogle :

You may not always want it to auto indent.

@fruggiero
Copy link
Contributor Author

fruggiero commented Dec 1, 2015

Bump.

I think is easy to review

@benogle
Copy link
Contributor

benogle commented Dec 1, 2015

@benogle benogle added the atom label Dec 1, 2015
@fruggiero
Copy link
Contributor Author

fruggiero commented Jan 19, 2016

Rebased on current master.
Any update to this review? @maxbrunsfeld

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Jan 19, 2016

Yeah, this makes a lot of sense; great suggestion, @fruggiero.

maxbrunsfeld pushed a commit that referenced this pull request Jan 19, 2016
bugfix: auto indentation after suggestion confirm
@maxbrunsfeld maxbrunsfeld merged commit 5dfb9fd into atom:master Jan 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants