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

No docs in code completion #23694

Closed
bwilkerson opened this issue Jun 22, 2015 · 13 comments
Closed

No docs in code completion #23694

bwilkerson opened this issue Jun 22, 2015 · 13 comments
Assignees
Labels
analyzer-completion Issues with the analysis server's code completion feature area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. customer-flutter P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@bwilkerson
Copy link
Member

The CompletionSuggestion should have a value for the docSummary and docComplete fields when the source code has documentation, but they don't. For example, asking for code completion at the end of the next-to-last line in this sample code will return null (missing) fields:

/// This is a dummy method.
///
/// But I'm no dummy!
noDoc() {
}

main() {
  noDo
}
@bwilkerson bwilkerson added Type-Defect area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-completion Issues with the analysis server's code completion feature labels Jun 22, 2015
@devoncarew
Copy link
Member

I ran into this as well; it would be great to be able to preview dartdoc info when completing methods.

@scheglov
Copy link
Contributor

I remember that:

  1. In Dart Editor we decided to use analysis.getHover to get documentation.
  2. Couple months ago we discussed this again and... I don't remember the resolution, but I remember that @lukechurch was concerned about amount of data.

I'm also concerned about amount of work we would need to perform to extract documentation for each CompletionSuggestion.

Also, could one of the clients remind me what is, or what do they expect to see in docSummary?
What's the difference between it and method signature, which can be composed using element?
@danrubel , do you remember?

@devoncarew
Copy link
Member

docSummary would I assume be the first markdown paragraph from the dartdoc documentation. The Atom client will likely never use the docComplete field. Perhaps that field could be deprecated.

I'm also concerned about amount of work we would need to perform to extract documentation for each CompletionSuggestion.

Can we implement it, and measure the actual cost? If we do see a significant cost to populating the field, this could be something that clients opt into.

@scheglov
Copy link
Contributor

scheglov commented Oct 1, 2015

@scheglov
Copy link
Contributor

scheglov commented Oct 1, 2015

I'm coping this performance difference from the CL.

For the first time when we complete top-level elements, such as "Str^" it takes
about 500-600 ms vs. 300 ms without documentation.
For the second and subsequent completions difference is not so high, but still
exists.

For prefixed completions difference is pretty high - 15 ms vs. 3 ms for
stringLiteral.^ completion.
I.e. documentation makes it 5 times more expensive.

I'm not sure that we want to take such performance hit.```

@scheglov
Copy link
Contributor

scheglov commented Oct 2, 2015

It is still slow to compute and not put into JSON.
So, I'm trying to optimize its computation.
https://codereview.chromium.org/1376473007

scheglov added a commit that referenced this issue Oct 4, 2015
@scheglov
Copy link
Contributor

scheglov commented Oct 4, 2015

Done.
Performance is still not perfect though.

#23694

@scheglov scheglov closed this as completed Oct 4, 2015
@scheglov
Copy link
Contributor

scheglov commented Oct 4, 2015

Err...
1af7b15

@scheglov
Copy link
Contributor

scheglov commented Oct 4, 2015

It's very slow during switching tabs.
Rolling back...

@clayberg clayberg reopened this Oct 5, 2015
@devoncarew devoncarew added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed Priority-Medium labels Nov 30, 2015
@devoncarew devoncarew assigned pq and unassigned scheglov Dec 10, 2015
@devoncarew
Copy link
Member

Switching to @pq; Phil, let's meet about this - this is the last big item left.

pq added a commit that referenced this issue Dec 16, 2015
Related to profiling the cost of storing element docs but the element counts themselves are generally interesting.

Background: #23694

R=brianwilkerson@google.com

Review URL: https://codereview.chromium.org/1537503002 .
@pq
Copy link
Member

pq commented Dec 17, 2015

A bit of profiling has confirmed that caching doc comment contents with elements shouldn't add too much overhead. (Needless to say, this too will benefit from summaries.)

For example, this shows the number of elements, docs and char counts for a flutter app and the flutter lib. Adds up to ~500k.

screen shot 2015-12-17 at 2 06 25 pm

@pq
Copy link
Member

pq commented Dec 17, 2015

pq added a commit that referenced this issue Dec 18, 2015
This does away with the expensive call to `computeDocumentationComment` in favor of cached comments.  Notably this makes adding doc content to code completion proposals performant (and so is done here).  It should also make `dartdoc` *much* faster for doc generation since there are no more trips to disk to fetch comments  for elements (still needed for source though).

For more on the desire for docs in completions see here: #23694

R=brianwilkerson@google.com, scheglov@google.com

Review URL: https://codereview.chromium.org/1534043002 .
@pq
Copy link
Member

pq commented Dec 18, 2015

Landed in 1f8eddc.

@pq pq closed this as completed Dec 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-completion Issues with the analysis server's code completion feature area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. customer-flutter P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

7 participants