Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

Optional manual auto completion #313

Closed
wants to merge 2 commits into from
Closed

Conversation

Istarnion
Copy link
Contributor

I added in an option in config.json for doing manual auto completion, and a keybinding for it.
It was difficult to find a good place to hook up the keybinding to the action. First I tried to get it in the ClangViewAutocomplete constructor, but accelerators are global and I couldn't get them to act on the currently active view. It might be possible, but I don't know GTK well enough.
Second attempt was in the Notebook constructor, but that is called before the config is loaded, so that didn't work either.
In the end, I just put it in juci.cc, in the on_activate function. This is probably not where it should be, but it works now. Any ideas where it might be better suited? Or is there a better way to hook up keybindings all together?

Also, I'm not sure what happened to the submodules.. I think they just got updated. I haven't edited anything in them atleast.

@eidheim
Copy link
Member

eidheim commented Feb 5, 2017

Thank you, this helps a lot. Even though I will probably make another commit based on this PR due to the submodule issue. I'll make you author on the new commit if that is ok. I'll also clean up here and there after.

@Istarnion
Copy link
Contributor Author

Super! Glad I could help

@eidheim
Copy link
Member

eidheim commented Feb 9, 2017

@Istarnion this functionality should now be implemented in eidheim@ee7efc8. Could you have a look and test it? I'll wait for your response before merging to cppit/jucipp master.

@Istarnion
Copy link
Contributor Author

I cloned your fork, and it ran exactly as expected. Thanks. Having it as only one option in the config file was alot better :)
I tested the same edge conditions as I did before, and there were no issues.

@eidheim
Copy link
Member

eidheim commented Feb 9, 2017

Thank you again! The commit has been merged. Closing this PR then.

@eidheim eidheim closed this Feb 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants