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

PHP: Show documentation (ctrl+shift+space) displays incorrect documentation #4494

Closed
troizet opened this issue Aug 10, 2022 · 7 comments · Fixed by #5701
Closed

PHP: Show documentation (ctrl+shift+space) displays incorrect documentation #4494

troizet opened this issue Aug 10, 2022 · 7 comments · Fixed by #5701
Assignees
Labels
kind:bug Bug report or fix PHP [ci] enable extra PHP tests (php/php.editor)
Milestone

Comments

@troizet
Copy link
Collaborator

troizet commented Aug 10, 2022

Apache NetBeans version

Apache NetBeans 14

What happened

It looks like the documentation search for the display is not done by the full word, but by a few characters before the caret. Also the search is done for partial matches.

How to reproduce

simplescreenrecorder-2022-08-10_22.11.46.mp4

Did this work correctly in an earlier version?

No / Don't know

Operating System

Ubuntu 20.04.4 LTS

JDK

OpenJDK 64-Bit Server VM 11.0.16+8-post-Ubuntu-0ubuntu120.04

Apache NetBeans packaging

Apache NetBeans provided installer

Anything else

Same behaviour on Apache NetBeans 15 release candidate 3.

Are you willing to submit a pull request?

No

Code of Conduct

Yes

@troizet troizet added kind:bug Bug report or fix needs:triage Requires attention from one of the committers labels Aug 10, 2022
@troizet troizet changed the title PHP: ctrl+shift+space displays incorrect documentation PHP: Show documentation (ctrl+shift+space) displays incorrect documentation Aug 11, 2022
@mbien mbien added the PHP [ci] enable extra PHP tests (php/php.editor) label Aug 12, 2022
@junichi11
Copy link
Member

Could you please provide an example code to reproduce it?

@troizet
Copy link
Collaborator Author

troizet commented Oct 3, 2022

You can check on the example of the standard function is_numeric():

  1. Write is_numeric()
  2. Set a caret between is_nu and meric()
  3. Click Source -> Show documentation
  4. The documentation for is_null() will be displayed
  5. Set the caret between is_num and eric()
  6. Click Source -> Show documentation.
  7. The documentation for is_numeric() will be displayed

@junichi11 junichi11 self-assigned this Oct 6, 2022
@junichi11 junichi11 removed the needs:triage Requires attention from one of the committers label Oct 6, 2022
@junichi11
Copy link
Member

I can't reproduce it...

My environment:

Product Version: Apache NetBeans IDE 15
Java: 11.0.16; OpenJDK 64-Bit Server VM 11.0.16+8-post-Ubuntu-0ubuntu120.04
Runtime: OpenJDK Runtime Environment 11.0.16+8-post-Ubuntu-0ubuntu120.04
System: Linux version 5.15.0-48-generic running on amd64; UTF-8;

nb-gh-4494

@troizet
Copy link
Collaborator Author

troizet commented Oct 12, 2022

I have checked on two environments on NetBeans versions starting from 12.6, the same behavior everywhere.

Product Version: Apache NetBeans IDE 15
Java: 11.0.16; OpenJDK 64-Bit Server VM 11.0.16+8-post-Ubuntu-0ubuntu120.04
Runtime: OpenJDK Runtime Environment 11.0.16+8-post-Ubuntu-0ubuntu120.04
System: Linux version 5.15.0-50-generic running on amd64; UTF-8; ru_RU (nb)
Product Version: Apache NetBeans IDE 15
Java: 11.0.16; OpenJDK 64-Bit Server VM 11.0.16+8-post-Debian-1deb10u1
Runtime: OpenJDK Runtime Environment 11.0.16+8-post-Debian-1deb10u1
System: Linux version 4.19.0-22-amd64 running on amd64; UTF-8; en_US (nb)

@troizet
Copy link
Collaborator Author

troizet commented Oct 12, 2022

I tried to understand the code.
The code that runs to display the documentation

void resolveDocumentation(ParserResult controller) throws IOException {
if (element != null) {
documentation = GsfCompletionDoc.create(controller, element, new Callable<Boolean>(){
@Override
public Boolean call() throws Exception {
return isTaskCancelled();
}
});
} else {
Env env = getCompletionEnvironment(controller, false);
int offset = env.getOffset();
String prefix = env.getPrefix();
results = new ArrayList<CompletionItem>();
isTruncated = false;
isFilterable = true;
anchorOffset = env.getOffset() - ((prefix != null) ? prefix.length() : 0);
CodeCompletionHandler completer = env.getCompletable();
if (completer != null) {
CodeCompletionContext context = new CodeCompletionContextImpl(offset, controller, prefix, false, QueryType.DOCUMENTATION);
CodeCompletionResult result = completer.complete(context);
if (result == null) {
Logger.getLogger(this.getClass().getName()).log(Level.WARNING, completer.getClass().getName() + " should return CodeCompletionResult.NONE rather than null");
result = CodeCompletionResult.NONE;
}
if (result != CodeCompletionResult.NONE) {
for (CompletionProposal proposal : result.getItems()) {
ElementHandle el = proposal.getElement();
if (el != null) {
documentation = GsfCompletionDoc.create(controller, el, new Callable<Boolean>(){
@Override
public Boolean call() throws Exception {
return isTaskCancelled();
}
});
// TODO - find some way to show the multiple overloaded methods?
if (documentation != null && documentation.getText() != null && documentation.getText().length() > 0) {
// Make sure we at least pick an alternative that has documentation
break;
}
}
}
}
}
}
}

Here we get the ElementHandle for which we want to display the documentation

CodeCompletionResult result = completer.complete(context);

Judging by the code, PHPCodeCompletion

public class PHPCodeCompletion implements CodeCompletionHandler2 {

doesn't use prefix, prefixMatch and queryType.DOCUMENTATION parameters passed to CodeCompletionContext.

And, in the is_numeric example, where caret is set between is_nu and meric(), instead of using prefix = is_numeric, prefix = is_nu is used.
Therefore, instead of a specific ElementHandle for is_numeric, PHPCodeCompletion returns a list of suggested ElementHandles for is_nu.
In this case, is_null and is_numeric, where is_null is first in the list, for which the documentation is displayed

It looks like we need to modify PHPCodeCompletion so that it uses the prefix, prefixMatch and queryType.DOCUMENTATION parameters passed to CodeCompletionContext.

@fyodorovich
Copy link

I see the same behaviour as @troizet. However, I regard this as a desirable UI behaviour. It allows the user a lot of flexibility. The displayed list is responsive to user input and allows users to obtain the correct function name simply by typing a few letters, then selecting from the list. It is an excellent didactic tool. It increases my knowledge and understanding of the language. I don't want this feature to change.

junichi11 added a commit to junichi11/netbeans that referenced this issue Mar 22, 2023
- apache#4494
- Add a new test method for checking documentation when a `QueryType` is `DOCUMENTATION` to `CslTestBase`
- If `QueryType` is `DOCUMENTATION`, get a whole identifier as a prefix
- Add unit tests
junichi11 added a commit to junichi11/netbeans that referenced this issue Mar 22, 2023
- apache#4494
- Add a new test method for checking documentation when a `QueryType` is `DOCUMENTATION` to `CslTestBase`
- If `QueryType` is `DOCUMENTATION`, get a whole identifier as a prefix
- Add unit tests
@junichi11
Copy link
Member

I can reproduce it with an example of the PR. Thanks.

@junichi11 junichi11 added this to the NB18 milestone Mar 23, 2023
@junichi11 junichi11 linked a pull request Mar 23, 2023 that will close this issue
junichi11 added a commit to junichi11/netbeans that referenced this issue Mar 25, 2023
- apache#4494
- Add a new test method for checking documentation when a `QueryType` is `DOCUMENTATION` to `CslTestBase`
- If `QueryType` is `DOCUMENTATION`, get a whole identifier as a prefix
- Add unit tests
- Increase spec versions
tmysik added a commit that referenced this issue Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug Bug report or fix PHP [ci] enable extra PHP tests (php/php.editor)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants