Feedback on "closing labels" preview feature

v2.3.1 includes a preview feature enabled with the dart.previewFlutterCloseTagDecorations: true setting. This adds "closing tag" decorations to the closing parens of constructor calls within build methods.

This feature is behind a flag because it's incomplete, has sub-optimal performance and may not work reliably. It is being shipped solely for feedback on the idea and may require analyzer work in order to support in a reliable way.

Please post your feedback below!

Known issues:

These are things that (probably) can't be addressed without support from the analyzer.

[Perf] Doesn't scale well due to reliance on Outlines, Highlights and repeated getHover calls
Currently identifies build methods based on return type string (Widget)
Only shows constructors, doesn't show end of lists (children, actions)

Suggestions:

These are things that can be improved in the preview.

Support const as well as new
Prefix with // instead of / so looks a bit like a comment marking the end
Improve styling
Fix flicker when typing quickly (or holding keys) caused by not being able relate analyzer notifications with document versions - just keep resetting a timeout and only update after there's been no update for x milliseconds (where x > expected latency for the update)
Show decorations in any Widget-returning method
Don't show annotations against characters other than ) to mitigate out-of-sync notifications
Allow color to be customised rather than reusing an existing style

Could we recognize "const Icon" ?

Could we recognize "const Icon" ?

Oops, didn't spot that! Should be easy; currently I'm using the new to detect a constructor, didn't think about const. Obviously with real analyzer support we wouldn't have wonky string checks on keywords, but I'll tweak this in the next release.

I use IntelliJ, and I'm jealous we don't have this feature! Really cool idea :)

@sethladd

I can't take credit for it, @sethladd emailed me asking how possible it'd be 😄

I use IntelliJ, and I'm jealous we don't have this feature!

There's a solution to that ;-P

That said; this is behind a flag for a reason.. it has to send many requests to the analyzer to work; it really needs real analyzer support to become reasonable to ship without a flag/warning - whether it gets that will likely depend on the feedback.

@DanTup

@DanTup rocks for the quick turnaround!

And, @xster was the inspiration.

We have a bunch of VS Code + Flutter users here at Google, we'll ask them to turn on the flag and report feedback. We're also curious if others find this useful. If so, we can make a case to the analyzer team to support (or, help someone land the patch).

Cheers!

Another idea, close named arguments with the argument name rather than constructor type (or maybe both?)

Example:

new AlertDialog(
  title: new Text(
    'some'
    'complicated'
    'expression'
  )/title,  // <== this
)

Options:

/title
/title Text

I wonder if parsing Dart code directly using the analyzer API would be faster than sending requests to the analysis server. You shouldn't need resolved AST for this. dartfmt does this and works on a file-by-file basis. Perhaps parse the code (with some debounce strategy), find all NamedExpressions and, if it is multiple lines long, add a closing tag to it.

How can I install v2.4? I'd love to try this out.

close named arguments with the argument name rather than constructor type

not a bad idea! That matches how I've used commenting end tags in HTML.

Strawman:

new AlertDialog(
  title: new Text(
    'some'
    'complicated'
    'expression'
  )/Text(title),  // <== this
)

@yjbanov

@yjbanov

Another idea, close named arguments with the argument name rather than constructor type (or maybe both?)

I think both could get noisy, but the argument name could work. Problem is, if it's not named then we'd have to use the type, and then it might be inconsistent. Interested to hear what other people think though!

I wonder if parsing Dart code directly using the analyzer API would be faster than sending requests to the analysis server.

Currently Dart Code is written in TypeScript, so using the analyzer package would likely be non-trivial. I don't think performance is a big deal here; the current prototype seems to perform ok to me, and it's doing TERRIBLE things. If it was made a first-class request in the analyzer, I think it'd easily be fast enough - I mean code-completion talks to the analyzer and works fine (AFAIK) and this is far less time-sensitive than that :-)

@yyoon

How can I install v2.4? I'd love to try this out.

Whoops! I shipped this earlier than planned, so I called it v2.3.1 because there wasn't a lot else in it. The latest version on the marketplace (v2.3.1 - which Code probably already auto-updated to if you have Dart Code installed) has this; just set the setting mentioned at the top. Sorry for the confusion!

@DanTup

@DanTup I see. Now I can see it working by manually setting it to true. Thanks!

I've shipped Dart Code v2.4.0. This includes an implementation of "closing tags" that uses the analysis server (you must be using at least 1.25.0-dev.11.0 which was released yesterday; flutter is also using it if you update).

It's still behind a flag (though there's a new flag name dart.previewClosingLabels since it's not flutter-specific - though the old flag still works) to get some more feedback (it now appears for all Dart!).

Please try it out :-)

In its SDK-supported form, I love that the tags show up everywhere, including useful places like setState calls, and inside of functions that don't necessarily return a widget, but do work with widgets. It's one of those subtle tools that you won't realize how helpful it is until its gone.

Minor UI issue: noticed it was possible to momentarily type text behind the labels.

@xster

I've been away for two weeks and thought I'd return to some feedback on these labels appearing in twoo many places, but there hasn't been any. For anyone that's used this seriously, does it seem reasonable to drop the flag and ship to everyone?

(I will first investigate @xster's bug, which I've opened #423 for).

We showed it off at GDD Europe :)

@sethladd

@sethladd Neat! Was there any feedback? (or was it in a recorded demo?).

@devoncarew You had concerns it might appear in too many places - have you had chance to try it out properly and see if it still feels that way?

Hey - just cycling back to this now. We've taken a look locally, and do think that it's slightly heavy in the number of places it shows closing tags. We'd need to spot check more code, but at a first blush, list and method closing tags might be too noisy (constructor closing tags are great). For list closing tags, one thing that may help is if the closing tag looks like the declaration (currently it looks like <Widget>[... and // List<Widget>, which is correct from a semantic POV, but it may be easier to line things up with your eye if we change the closing tag to be // <Widget>[]).

We'll do some more investigation here, and when we're further along run it by UXR.

My 2 cents so far:

Thank god you made this!

Perhaps the only tweak is that sometimes it visually blends in with actual comments a bit too much. Could we make it a smaller font or give it some other distinguishing visual treatment?

Either way, low priority. I think it's great as is.

@xster

This is more of a meta-feedback. I'll definitely file feedback about the labels themselves when I see something that hasn't been discussed yet. My feedback is about the feedback banner itself. Is there a way to disable it, or have it pop up only once in a while (e.g. once a week)? I jump from project to project a lot, and it pops up every time.

Also, joining @xster. I 😍 this feature!

@devoncarew

@devoncarew Thanks for the info. Give me a shout when you have some solid changes you want to make (or if easier for for you make them, feel free). I haven't removed the flag from this yet as I thought 1.25 was near and wanted to do a final test with the release SDK before doing so.

@xster Glad you like it! We can customse things, but smaller fonts might look a bit weird. The colour is customisable already, though I don't know if that's enough. It's possible to add borders/underlines/etc., but I think they'll make it less subtle, and probably more subtle is what you're after? If you have ideas, I can try them out and make screenshots (or if you're comfortable running Dart Code from source you can try it yourself - relevant code is this and Code docs on what's possible are here.

@yjbanov Not currently; but I've been meaning to remove it for a while. I saw someone tweet about uninstalling an extension that was doing something like this and I realised how annoying it is. I should've made it only show once :(

If 1.25 SDK ships soon, I'll remove the flag so it'll go anyway. If not, I'll push an update out to remove it soon. Sorry for the annoyance - I haven't used it much myself (been away for 3 weeks) so it hasn't been on my mind ;(

@yjbanov

@yjbanov

My feedback is about the feedback banner itself. Is there a way to disable it, or have it pop up only once in a while (e.g. once a week)?

v2.4.2 removes this banner (it's the only change). Sorry for the annoyance; I really should've made it only appear now and then (esp. since the extension already has some only-shown-once popups!).

I've finally found something that isn't absolutely perfect about this feature.

When folding a code section that contains a closing tag, the closing tag is added to the front of the fold.

I'm not sure how useful this behavior is, so please correct me if I'm wrong.
My personal expectation is to have the tag hidden until I decide to unfold.

Screenshot

Yeah, that looks crap. Can you raise a specific issue for it? Dunno if we can easily fix before MS overhaul decorations though 😔

Closing this as this is done. We still have #445 which people are directed to if they turn closing labels off. Analytics say 99% of people have it enabled.

Navigation Menu

Repository navigation

Feedback on "closing labels" preview feature #383

Known issues:

Suggestions:

23 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Feedback on "closing labels" preview feature #383

Description

Known issues:

Suggestions:

Activity

sethladd commented on Jul 23, 2017

DanTup commented on Jul 23, 2017

brianegan commented on Jul 23, 2017

DanTup commented on Jul 23, 2017

sethladd commented on Jul 23, 2017

yjbanov commented on Jul 24, 2017

yyoon commented on Jul 24, 2017

sethladd commented on Jul 24, 2017

DanTup commented on Jul 24, 2017

yyoon commented on Jul 24, 2017

23 remaining items

DanTup commented on Aug 17, 2017

Skylled commented on Aug 18, 2017

xster commented on Aug 23, 2017

DanTup commented on Sep 7, 2017

sethladd commented on Sep 7, 2017

DanTup commented on Sep 7, 2017

devoncarew commented on Sep 20, 2017

xster commented on Sep 20, 2017

yjbanov commented on Sep 21, 2017

DanTup commented on Sep 21, 2017

DanTup commented on Sep 22, 2017

Skylled commented on Oct 5, 2017

DanTup commented on Oct 5, 2017

DanTup commented on Feb 21, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions