Support click on inlayhint label parts

[BoykoAlex]

InlayHint may have a number of InlayHintLabelParts. This is necessary support to determine which label part has been clicked on to execute the command corresponding to the clicked label part.

Includes a test for a single label part

[BoykoAlex]

What do you think of this PR @mickaelistria @rubenporras ? I wasn't able to write a proper unit test for the multi part label... However, testing it manually with multi part label from LS each part with its own command - worked very well for me.
I've tried to find GEF/GMF DirectEditRequest logic that would find from the click where to place the cursor in the popped up text cell editor (i.e. find char under mouse) but looks like this code is gone... Nevertheless, what I've implemented seem to do the job... GC would only be created if more than one label part present and multi-label clicking seem to perform acceptable.

[rubenporras]

should we not use LanguageServerDocumentExecutor.LanguageServerDocumentExecutor(IDocument) and execute the command through it so that we ensure the order of the executions for the given document?

[BoykoAlex]

Shall I change it to:

	public final Consumer<MouseEvent> getAction() {
		return me -> {
			String title= getLabel();
			if (title != null && !title.isEmpty()) {
				findLabelPart(me).ifPresent(labelPart -> {
					final Command command = labelPart.getCommand();
					if(command != null && command.getCommand() != null && !command.getCommand().isEmpty()) {
						ExecuteCommandOptions provider = wrapper.getServerCapabilities().getExecuteCommandProvider();
						if (provider != null && provider.getCommands().contains(command.getCommand())) {
							LanguageServerDocumentExecutor.forDocument(document).computeAll((wrapper, ls) -> {
								if (wrapper == this.wrapper) {
									return ls.getWorkspaceService()
											.executeCommand(new ExecuteCommandParams(command.getCommand(), command.getArguments()));
								}
								return CompletableFuture.completedFuture(null);
							});
						} else  {
							CommandExecutor.executeCommandClientSide(command, document);
						}
					}
				});
			}
		};
	}

Would this be the recommended way you had in mind @rubenporras ?

[rubenporras]

Sorry for having missed your comment. Yes, I think something like that is good. Maybe use just (w, ls) instead of (wrapper, ls), as I think that is the commonly used abbreviation in the codebase.

[mickaelistria]

IMO, explicit names that one can read out of a context are always better than abbreviation which require deep knowledge of the overall code. I think we should avoid mandating such "shortening" of names in general and most often stick with the contributor's style. I find project that welcome and handle more diverse style are more comfortable to work with than the ones that force a style (Maven is my worst experience in OSS contribution because of that)

[rubenporras]

Yes, I agree that they are usually better. Only this context, wrapper already exists as a class field, so I would tend to give a different name if possible. In any case, it was only a suggestion, not a requirement :)

[mickaelistria]

Only this context, wrapper already exists as a class field,

Ah OK, I had missed that point.

[BoykoAlex]

Thank you! Updated as requested :-)

[BoykoAlex]

Any update on this PR? Shall I indeed use LanguageServerDocumentExecutor.LanguageServerDocumentExecutor(IDocument) to execute a command from inlay label part?

[BoykoAlex]

@mickaelistria @rubenporras am I okay to merge this PR? I see the build is failing but if i run it locally on master it is the same failure :-\

[rubenporras]

@BoykoAlex , if you rebase, it should build again.

[rubenporras]

I only have minor comments. Would you like to merge as it is or would you like to modify the PR a little bit?

[rubenporras]

would that be a bit more readable by reducing the number of nested ifs?

Suggested change

	findLabelPart(me).ifPresent(labelPart -> {
	findLabelPart(me).map(InlayHintLabelPart::getCommand).map(Command::getCommand).ifPresent(command -> {

[BoykoAlex]

Sounds good, but simplified it up to map(InlayHintLabelPart::getCommand) since both commandId, i.e. command.getCommand() and the command itself are required in the black below.

[rubenporras]

Suggested change

	for (InlayHintLabelPart l : inlayHint.getLabel().getRight()) {
	for (InlayHintLabelPart labelPart : inlayHint.getLabel().getRight()) {

[BoykoAlex]

Sounds good, did this.

[rubenporras]

Could we extract inlayHint.getLabel().getRight() to a local variable and replace the three accesses in this method?

Suggested change

	if (inlayHint.getLabel().getRight().size() == 1) {
	if (inlayHint.getLabel().getRight().size() == 1) {

[BoykoAlex]

Sounds good, did this.