Completion items are not sorted when extra text typed after completion invocation

[BoykoAlex]

The LSContentAssistProcessor sorts completions based on the rank when completions are first computed. If extra chars are typed after the initial CA invocation then the list of proposals is filtered (as invalid proposals are filtered out) but never sorted again based on the document filter text. The generic editor ContentAssistant is null thus nothing is sorted after the initial invocation. The result is irrelevant completion proposals ends up at the top of the list.
It'd be great if somehow we could set the sorter on the ContentAssistant... I couldn't find a way unfortunately and ended up using reflection just to test the hypothesis.
After writing the code below in lsp4e i was able to achieve the desired effect:

	private void installSorterIfAbsetnt(ITextViewer viewer) {
		if (viewer instanceof SourceViewer) {
			try {
				Field f = SourceViewer.class.getDeclaredField("fContentAssistant"); //$NON-NLS-1$
				f.setAccessible(true);
				Object o = f.get(viewer);
				if (o instanceof ContentAssistant) {
					ContentAssistant contentAssitant = (ContentAssistant) o;
					Field sorterField = ContentAssistant.class.getDeclaredField("fSorter"); //$NON-NLS-1$
					sorterField.setAccessible(true);
					if (sorterField.get(contentAssitant) == null) {
						contentAssitant.setSorter(new ICompletionProposalSorter() {

							@Override
							public int compare(ICompletionProposal p1, ICompletionProposal p2) {
								if (p1 instanceof LSCompletionProposal && p2 instanceof LSCompletionProposal) {
									return proposalComparator.compare((LSCompletionProposal) p1, (LSCompletionProposal) p2);
								}
								return 0;
							}
						});
					}
				}
			} catch (Exception e) {
				// ignore
			}
		}
	}

	@Override
	public ICompletionProposal[] computeCompletionProposals(ITextViewer viewer, int offset) {
		installSorterIfAbsetnt(viewer);

		IDocument document = viewer.getDocument();

                ....

I'd be more than happy to provide a patch if someone points in the right direction to implement this elegantly :-)

[mickaelistria]

One possible workaround can probably be to use isIncomplete: true int the completion results, so LSP4E would then re-invoke, and hopefully re-sort, the results.

[mickaelistria]

The generic editor ContentAssistant is null thus nothing is sorted after the initial invocation.

Isn't it the root issue?

[rubenporras]

While isIncomplete: true will probably work, we should avoid it, as it will cause additional roundtrips.

Not being able to reorder is an usability problem, so it would be nice if someone would have time to fix it. Maybe it requires a platform change?

[BoykoAlex]

@mickaelistria isIncomplete: true was the first thing I've tried but it didn't work. After some digging i think i see why it didn't work for me.

isIncomplete only checked here:

	@Override
	public boolean isValidFor(IDocument document, int offset) {
		return (!isIncomplete || offset == this.initialOffset) && validate(document, offset, null);
	}

but looking at the code in CompletionProposalPopup classes i see this:

			if (proposal instanceof ICompletionProposalExtension2) {

				ICompletionProposalExtension2 p= (ICompletionProposalExtension2) proposal;
				try {
					if (p.validate(document, offset, event))
						filtered.add(proposal);
				} catch (RuntimeException e) {
					// Make sure that poorly behaved completion proposers do not break filtering.
				}
			} else if (proposal instanceof ICompletionProposalExtension) {

				ICompletionProposalExtension p= (ICompletionProposalExtension) proposal;
				try {
					if (p.isValidFor(document, offset))
						filtered.add(proposal);
				} catch (RuntimeException e) {
					// Make sure that poorly behaved completion proposers do not break filtering.
				}
			} else {
				// restore original behavior
				fIsFilteredSubset= false;
				fInvocationOffset= offset;
				fContentAssistant.fireSessionRestartEvent();
				fComputedProposals= computeProposals(fInvocationOffset);
				return fComputedProposals;
			}

Thus it never gets to p.isValidFor(document, offset) but instead calls p.validate(document, offset, event)

I put the following code in LSCompletionProposal and then it worked properly:

	@Override
	public boolean validate(IDocument document, int offset, DocumentEvent event) {
		if (item.getLabel() == null || item.getLabel().isEmpty()) {
			return false;
		}
		if (offset < this.bestOffset) {
			return false;
		}
		try {
			String documentFilter = getDocumentFilter(offset);
			if (!documentFilter.isEmpty()) {
				return /* CHANGE START*/!(isIncomplete && currentOffset != initialOffset) && /* CHANGE END */ CompletionProposalTools.isSubstringFoundOrderedInString(documentFilter, getFilterString());
			} else if (item.getTextEdit() != null) {
				return offset == LSPEclipseUtils.toOffset(getTextEditRange().getStart(), document);
			}
		} catch (BadLocationException e) {
			LanguageServerPlugin.logError(e);
		}
		return true;
	}

[sebthom]

@BoykoAlex can we close this issue?

[BoykoAlex]

Hmm... I think so, but let me check this out first and get back to you

[BoykoAlex]

Looks like the merged PR is only to address the support for isIncomplete: true. The issue is that we need completion proposals sorter for the isIncomplete: false as well. Seems that a platform change is required to get this fixed.

[martinlippert]

Does anyone have a pointer here where the isIncomplete case is handled or - to be more precise - where the new attempt to grab an updated list of proposals from the language server is triggered after the user typed more characters?