Skip to content
This repository has been archived by the owner on Jun 20, 2019. It is now read-only.

Conversation

MichaelRFairhurst
Copy link
Contributor

This won't solve the problem by itself without updating the SDK (PR incoming)...but I'm also curious if this passes on travis, in which case we can commit it to master without issue and support both versions (internal/external) without branching.

When autocompleting a dart snippet like "x" within "{{}}", there is no
parent node and that breaks some logic in the completion engine.
Therefore we add a synthetic parenthesized ast, but previously we had
been giving null begin and end tokens.

Create begin and end tokens via the front-end synthetic classes, and
ensure to link beginToken.next to the user's dart code's beginToken.

When autocompleting a dart snippet like "x" within "{{}}", there is no
parent node and that breaks some logic in the completion engine.
Therefore we add a synthetic parenthesized ast, but previously we had
been giving null begin and end tokens.

Create begin and end tokens via the front-end synthetic classes, and
ensure to link beginToken.next to the user's dart code's beginToken.
Copy link
Contributor

@stereotype441 stereotype441 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -9,6 +9,8 @@ import 'package:angular_analyzer_plugin/ast.dart';
import 'package:angular_analyzer_plugin/src/converter.dart';
import 'package:angular_analyzer_plugin/src/model.dart';
import 'package:angular_analyzer_plugin/src/standard_components.dart';
import 'package:front_end/src/scanner/token.dart'
show SyntheticBeginToken, TokenType, SyntheticToken;
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the analyzer re-exports TokenType via package:analyzer/dart/ast/token.dart so ideally it would be better to import it from there (we are trying to avoid having too many packages depend on front_end while its API is unstable).

Same issue ought to apply to SyntheticBeginToken and SyntheticToken, except that...oops!...the analyzer doesn't re-export them. I'll make a change to the analyzer that causes it to re-export SyntheticBeginToken and SyntheticToken via package:analyzer/src/dart/ast/token.dart, and then once that lands we can change this import statement.

But for now I think it's fine to leave it as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, thanks!

@MichaelRFairhurst
Copy link
Contributor Author

Looks like I'll have to close this PR, since it doesn't integrate with the latest version of package:analyzer_plugin. But I'll make a different one that merges into my side branch...

dart-bot pushed a commit to dart-lang/sdk that referenced this pull request Apr 24, 2018
These classes are needed by the angular_analyzer_plugin project, and
we don't want to make it depend directly on front_end, so we need to
re-export the classes from the analyzer.  See discussion at
dart-archive/angular_analyzer_plugin#558 (comment)

Change-Id: Ide0264e3e04f783eb9f68079cc2de8a6acfb94b3
Reviewed-on: https://dart-review.googlesource.com/52425
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants