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

Use the GTK completion widget in CoqIDE #11400

Merged
merged 4 commits into from
Jan 16, 2020

Conversation

ppedrot
Copy link
Member

@ppedrot ppedrot commented Jan 15, 2020

Fixes #9945

@ppedrot ppedrot added kind: fix This fixes a bug or incorrect documentation. part: CoqIDE Issues and PRs related to CoqIDE or other IDE features of coq. labels Jan 15, 2020
@ppedrot ppedrot added this to the 8.11.1 milestone Jan 15, 2020
@ppedrot ppedrot requested a review from a team as a code owner January 15, 2020 09:09
Copy link
Member

@herbelin herbelin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there some parts of the code particularly to double-check? Trusting otherwise.

@ppedrot
Copy link
Member Author

ppedrot commented Jan 15, 2020

@eponier and @Alizter already commented on the bug report about the usability of this patch. Remaining issues that I don't know how to solve:

  • The completion pop-up does not have any border (@eponier)
  • Picks up words that shouldn't be considered (@Alizter)
  • The box appears over the text sometimes (@Alizter)
  • When pressing backspace it should go away (@Alizter)

These behaviours are hardwired in GtkSourceView, and we have no control over them AFAICT.

@eponier
Copy link
Contributor

eponier commented Jan 15, 2020

The point about the border is really a minor one.

@ppedrot
Copy link
Member Author

ppedrot commented Jan 16, 2020

I consider this ready. It could even be eligible for 8.11.0, but I am afraid this is lacking a larger test base.

@ejgallego
Copy link
Member

After testing, IMO this looks pretty good.

Even if we need to tweak some stuff, the PR cannot be worse than the current status so I'm going to merge.

@ejgallego ejgallego modified the milestones: 8.11.1, 8.11.0 Jan 16, 2020
ejgallego added a commit that referenced this pull request Jan 16, 2020
Reviewed-by: ejgallego
Reviewed-by: herbelin
@ejgallego ejgallego merged commit 17df0e7 into coq:master Jan 16, 2020
@ejgallego ejgallego mentioned this pull request Jan 16, 2020
1 task
@ppedrot ppedrot deleted the gtk-ide-completion branch January 17, 2020 10:07
@ppedrot
Copy link
Member Author

ppedrot commented Jan 19, 2020

In the end, I think I am going to backport this in 8.11.0. I'll test it a bit more before, but the answers from our aforementioned guinea pigs and my limited poking with a stick seemed to be supporting this.

@ejgallego
Copy link
Member

As discussed on gitter, I also think the backport is a good idea.

ppedrot added a commit to ppedrot/coq that referenced this pull request Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: fix This fixes a bug or incorrect documentation. part: CoqIDE Issues and PRs related to CoqIDE or other IDE features of coq.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoqIDE completion does not work on gtk3
4 participants