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

UX improvements to dartfix --list output #36875

Closed
devoncarew opened this issue May 7, 2019 · 5 comments
Closed

UX improvements to dartfix --list output #36875

devoncarew opened this issue May 7, 2019 · 5 comments
Labels
analyzer-dartfix Issues with the dartfix package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.

Comments

@devoncarew
Copy link
Member

The --list output for dartfix is:

Getting list of fixes...       0.0s

collection-if-elements
    Convert to using if elements when building collections.

double-to-int
    Find double literals ending in .0 and remove the .0
    wherever double context can be inferred.

fix-named-constructor-type-arguments (required)
    Move named constructor type arguments from the name to the type.

map-for-elements
    Convert to for elements when building maps from iterables.

non-nullable
    Experimental: Update sources to be non-nullable by default.
    Requires the experimental non-nullable flag to be enabled.
    This is not applied unless explicitly included.

use-mixin (required)
    Convert classes used as a mixin to the new mixin syntax.

use-spread-collections
    Convert to using collection spread operators.

We should likely sort so that the (required) items are at the top. We may want an 'experimental' section at the end (which would currently hold the nnbd option). We should also add a lot more detail to each fix, including more text about what it does, and a before and after example. We can make liberal use of indent, bold, and whitespace here to make these longer descriptions more legible.

cc @danrubel

@devoncarew devoncarew added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-dartfix Issues with the dartfix package labels May 7, 2019
@devoncarew devoncarew changed the title dartfix --list output UX improvements to dartfix --list output May 7, 2019
@devoncarew
Copy link
Member Author

Additionally, (required) is a confusing concept here. Perhaps (default), or, (enabled by default) is a better description?

@devoncarew
Copy link
Member Author

devoncarew commented May 7, 2019

It was difficult for me to figure out how to run the nnbd tooling here. Eventually I determined it was 'dart bin/dartfix.dart --include=non-nullable .'. I:

  • tried with -h
  • then ran with --list
  • saw the nnbd option, and figured out I need to use that id with --include
  • ran that, and got an error Expected at least one file or directory to analyze., so then passed in the cwd

We should iterate here a bit.

dart-bot pushed a commit that referenced this issue Jun 6, 2019
This removes this --list option from dartfix and instead displays
the list of fixes when the --help option is specified.

Partially addresses #36875

Change-Id: Ibd8b124c2c20a752f7e661672eac8aa2e4483b26
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105002
Auto-Submit: Dan Rubel <danrubel@google.com>
Reviewed-by: Devon Carew <devoncarew@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>
@danrubel
Copy link

danrubel commented Jun 6, 2019

This is addressed by

Now, either dartfix or dartfix --help will display the following text...

Usage: dartfix [options...] <directory paths>

Choosing fixes to be applied:
-i, --include=<name-of-fix>    Include a specific fix.
-x, --exclude=<name-of-fix>    Exclude a specific fix.
-r, --required                 Apply required fixes.

Modifying files:
-w, --overwrite                Overwrite files with the changes.
-f, --force                    Overwrite files even if there are errors.

Miscellaneous:
-h, --help                     Display this help message.
-v, --verbose                  Verbose output.
    --[no-]color               Use ansi colors when printing messages.

Listing fixes...

These fixes are automatically applied unless at least one
--include option is specified and --required is not specified.
They may be individually disabled using --exclude.

* fix-named-constructor-type-arguments
    Move named constructor type arguments from the name to the type.

* use-mixin
    Convert classes used as a mixin to the new mixin syntax.

These fixes are NOT automatically applied,
but may be enabled using --include.

* collection-if-elements
    Convert to using if elements when building collections.

* double-to-int
    Find double literals ending in .0 and remove the .0 wherever double context can be inferred.

* map-for-elements
    Convert to for elements when building maps from iterables.

* non-nullable
    Experimental: Update sources to be non-nullable by default.
    This requires the experimental non-nullable flag to be enabled.

* use-spread-collections
    Convert to using collection spread operators.

@srawlins
Copy link
Member

Ah yeah, looks complete.

@devoncarew
Copy link
Member Author

I think we can close this and open more specific issues as they come up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-dartfix Issues with the dartfix package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.
Projects
None yet
Development

No branches or pull requests

3 participants