Update FormatTool. -w bug fix and support language version#466
Update FormatTool. -w bug fix and support language version#466btr-rmconsole-2[bot] merged 6 commits intomasterfrom
Conversation
| ArgResults? argResults, | ||
| List<String>? configuredFormatterArgs, | ||
| bool passWriteArgForOverwrite = true, | ||
| String? languageVersion, |
There was a problem hiding this comment.
here's a non-formatting change
| defaultMode: defaultMode, | ||
| exclude: exclude, | ||
| formatter: formatter, | ||
| languageVersion: languageVersion, |
There was a problem hiding this comment.
here's a non-formatting change
| /// `--language-version`. | ||
| /// | ||
| /// If null, `latest` will be used when dart_dev decides the flag is needed. | ||
| String? languageVersion; |
There was a problem hiding this comment.
here's a non-formatting change
| FormatMode? mode, { | ||
| ArgResults? argResults, | ||
| List<String>? configuredFormatterArgs, | ||
| String? languageVersion, |
There was a problem hiding this comment.
here's a non-formatting change. added languageVersion
| // 1. Mode flag(s), if configured | ||
| if (mode == FormatMode.check) ...['-o', 'none', '--set-exit-if-changed'], | ||
| if (mode == FormatMode.dryRun) ...['-o', 'none'], | ||
| if (languageVersion != null) '--language-version=$languageVersion', |
There was a problem hiding this comment.
here's a non-formatting change
| FormatMode? defaultMode, | ||
| List<Glob>? exclude, | ||
| Formatter? formatter, | ||
| String? languageVersion, |
There was a problem hiding this comment.
here's a non-formatting change
| formatter, | ||
| dartStyleSupportsWriteArg, | ||
| configuredLanguageVersion: languageVersion, | ||
| ); |
There was a problem hiding this comment.
here's a non-formatting change. determine if we should use -w and what the language version to use is.
| mode, | ||
| argResults: context.argResults, | ||
| configuredFormatterArgs: configuredFormatterArgs, | ||
| languageVersion: formatterLanguageVersion, |
There was a problem hiding this comment.
pass in the language version
| argResults: context.argResults, | ||
| configuredFormatterArgs: configuredFormatterArgs, | ||
| passWriteArgForOverwrite: dartStyleSupportsWriteArg, | ||
| languageVersion: formatterLanguageVersion, |
There was a problem hiding this comment.
pass in language version and -w usage
| @@ -515,14 +580,99 @@ ProcessDeclaration buildFormatProcess([Formatter? formatter]) { | |||
| } | |||
| } | |||
|
|
|||
| /// | ||
| /// Unless [verbose] is true, the list of inputs will be abbreviated to avoid an | ||
| /// unnecessarily long log. | ||
| void logCommand( |
There was a problem hiding this comment.
the rest is formatting below here
| @@ -13,10 +13,10 @@ import '../utils/assert_no_positional_args_nor_args_after_separator.dart'; | |||
| /// Use [DevTool.fromFunction] to create [FunctionTool] instances. | |||
| class FunctionTool extends DevTool { | |||
There was a problem hiding this comment.
only formatting changes in this file
| @@ -28,19 +28,22 @@ class OverReactFormatTool extends DevTool { | |||
| Iterable<String> paths = context.argResults?.rest ?? []; | |||
There was a problem hiding this comment.
only formatting changes in this file
| @@ -27,12 +27,15 @@ final _log = Logger('Process'); | |||
| /// It is also possible to run this tool directly in a dart script: | |||
| /// ProcessTool(exe, args).run(); | |||
| class ProcessTool extends DevTool { | |||
There was a problem hiding this comment.
only formatting changes in this file
| @@ -50,48 +50,66 @@ class TestTool extends DevTool { | |||
| @override | |||
| final ArgParser argParser = ArgParser() | |||
| ..addSeparator('======== Selecting Tests') | |||
There was a problem hiding this comment.
only formatting changes in this file
| final ArgParser argParser = ArgParser() | ||
| ..addFlag('ignore-infos', help: 'Ignore any info level issues.'); | ||
|
|
||
| @override |
There was a problem hiding this comment.
only formatting changes in this file
| @@ -64,27 +64,40 @@ class WebdevServeTool extends DevTool { | |||
|
|
|||
| @override | |||
| final ArgParser argParser = ArgParser() | |||
There was a problem hiding this comment.
only formatting changes in this file
| @@ -0,0 +1,4 @@ | |||
| name: has_dart_style_v2 | |||
| @@ -0,0 +1,4 @@ | |||
| name: has_dart_style_v3 | |||
| orderedEquals(['a', 'b', '-w'])); | ||
| expect( | ||
| buildArgs(['a', 'b'], FormatMode.overwrite), | ||
| orderedEquals(['a', 'b', '-w']), |
| ); | ||
| }); | ||
|
|
||
| test('mode=overwrite without write arg', () { |
| ); | ||
| }); | ||
|
|
||
| test('adds latest language version flag when configured', () { |
There was a problem hiding this comment.
2 tests to test the new language version config
| ); | ||
| }); | ||
|
|
||
| test('adds latest language version flag when configured', () { |
| expect(execution.directiveOrganization, isNull); | ||
| }); | ||
|
|
||
| test( |
There was a problem hiding this comment.
new tests for dart_style
| final execution = buildExecution( | ||
| context, | ||
| configuredFormatterArgs: ['--fix', '--follow-links'], | ||
| formatter: Formatter.dartFormat, |
There was a problem hiding this comment.
updates to test for language version under dart 3
| test('<=5 inputs and verbose=false', () async { | ||
| expect(Logger.root.onRecord, | ||
| emitsThrough(infoLogOf(contains('dartfmt -x -y a b')))); | ||
| expect( |
There was a problem hiding this comment.
format changes below
| ); | ||
| }); | ||
|
|
||
| test('detects languageVersion', () { |
| String formatToolCascadeSrc({String formatter = 'dartfmt'}) => | ||
| String formatToolCascadeSrc({ | ||
| String formatter = 'dartfmt', | ||
| String? languageVersion, |
There was a problem hiding this comment.
add language version
| ); | ||
| }); | ||
|
|
||
| test( |
| @@ -0,0 +1,3 @@ | |||
| void main() { | |||
| ); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
get the language version from config
| failedToReconstructFormatterArgs, | ||
| failedToParseFormatterArgs, | ||
| failedToParseLineLength, | ||
| failedToParseLanguageVersion, |
| } | ||
| } | ||
| return null; | ||
| } |
robbecker-wf
changed the title
btr-rmconsole-6
bot
added
Merge Requirements Met
and removed
Merge Requirements Unmet
labels
|
QA +1 CI passes on linux/windows and dart 2 and 3 |
3 participants
Summary
In version 3 of dart_style the
-wargument was removed from the CLI but dart_dev still uses it under Dart 3 because it can resolve to the newer dart_style version. dart_style changelog link https://pub.dev/packages/dart_style/changelog#:~:text=Remove%20the%20old,the%20CLI%20options.While fixing this to support dart_style 3, I realized that the constructor for DartFormatter now takes a required languageVersion parameter and the CLI takes the
--languageVersionargument. So in the spirit of supporting dart_style 3+ properly, this PR now conditionally uses the-wargument when dart_style is < 3 and provides the ability to configure the language version all the way through from config.dart for the FormatToolnote: MOST code changes are dart formatting changes. I've comment the PR where there are non-formatting changes.
Documentation improvements:
doc/tools/format-tool.mdexplaining how to configure the formatter language version usingdart_dev.