-
Notifications
You must be signed in to change notification settings - Fork 579
Update cursor after quick fix, using new data from dart-services #1210
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Fixes #1122, albeit in a fairly naive way. Once this is landed, I need to make a couple new issues for things like automatically adding an appropriate selection and simultaneous editing of linked edit groups. |
/// provide an offset on the cursor's current line to represent the position | ||
/// to which the cursor should move. This field allows for a character count | ||
/// from the top of the file to be used instead. | ||
final int absoluteCursorPosition; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this would be a List<int>
and _completionHelper
be responsible for either using the first or all of the cursor positions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there are a few forms in which cursor info can be created by the analyzer:
- Completions have a relative cursor offset, so that some have the cursor start immediately afterward, and method name completions have it start in between the parens. These could likely be converted to use the absolute one.
- A single, absolute cursor position is used for a one-shot fix like "Move Widget Down."
- A series of post-application "linked edit groups" require repeated multi-cursor, multi-selection editing by the user.
The first of those already worked when we showed up to the project. The second maps nicely to a single, absolute position, which is what I've done here. The third is much more complicated and is largely out of scope for this particular issue. As a stopgap, I'm translating the lined edit group info provided by the analyzer into a single absolute cursor position so we don't give the user something completely useless, but (as you point out), it's not ideal and will need to be revisited as our polishing efforts continue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third is much more complicated and is largely out of scope for this particular issue
I agree, I was just suggesting we make the type a List so that #3 goes more smoothly when we get to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nits
Decoupled analyzer from API, tested with both the old and new versions of dart-services without issue. @johnpryan PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - can we have an issue to track https://github.com/dart-lang/dart-pad/pull/1210/files#r314914346 ?
Done: #1213 |
No description provided.