Migrate status #102785

GaryQian · 2022-04-28T21:50:41Z

flutter migrate status subcommand.

This command shows the diffs staged in the migrate working directory and the manifest of files to be changed.

Depends on #101937

flutter-dashboard · 2022-04-28T21:50:44Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

blasten · 2022-05-05T23:28:40Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+    Directory workingDirectory = project.directory.childDirectory(kDefaultMigrateWorkingDirectoryName);
+    final String? customWorkingDirectoryPath = stringArg('working-directory');
+    if (customWorkingDirectoryPath != null) {
+      if (customWorkingDirectoryPath.startsWith(fileSystem.path.separator) || customWorkingDirectoryPath.startsWith('/')) {


how does this work on Windows?

blasten · 2022-05-05T23:33:56Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+      'working-directory',
+      help: 'Specifies the custom migration working directory used to stage and edit proposed changes. '
+            'This path can be absolute or relative to the flutter project root.',
+      valueHelp: 'path',


should it mention the default directory?

Yes, ill add it in the help text!

Idk if this will be painful, but can we name this something else, maybe workspace-directory? I think this will be confusing with the current working directory of the tool invocation. In reading the code, I thought this field was a way to override the project directory.

Renamed to migrate_staging_dir and the parameter to staging-directory

blasten · 2022-05-05T23:34:54Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+      help: 'Shows the diff output when enabled. Enabled by default.',
+    );
+    argParser.addFlag(
+      'show-added-files',


what is the use case for this flag?

Many of these flags assist in spam-reduction to make it easier to find the diffs/info the user is actually looking for.

blasten · 2022-05-05T23:37:12Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+      valueHelp: 'path',
+    );
+    argParser.addFlag(
+      'diff',


what is the use case for disabling this flag?

The diffs can often be quite spammy, programmatic uses especially may not wish to have such an extensive output.

blasten · 2022-05-05T23:38:14Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+    );
+    argParser.addOption(
+      'project-directory',
+      help: 'The root directory of the flutter project.',


for simplicity, should it only assume the current directory is the project directory?

This tool is also intended to be used programmatically, where this option helps immensely.

It looks like if this is not provided we default to the current working directory, can we document this?

blasten · 2022-05-05T23:39:08Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{};
+
+  /// Manually marks the lines in a diff that should be printed unformatted for visbility.
+  final Set<int> _initialDiffLines = <int>{0, 1};


what do these lines contain?

These lines just include the file names being diffed for example:

diff --git a/Users/garyq/projects/vanillabase1226/android/app/src/main/res/values/styles.xml b/Users/garyq/projects/vanillabase1226/migrate_working_dir/android/app/src/main/res/values/styles.xml index 1f83a33..8dbda4a 100644

Can you flesh out the dartdoc with this example?

blasten · 2022-05-05T23:42:06Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+          int lineNumber = -1;
+          for (final String line in result.diff!.split('\n')) {
+            lineNumber++;
+            if (line.startsWith('---') || line.startsWith('+++') || line.startsWith('&&') || _initialDiffLines.contains(lineNumber)) {


what happens if a line starts with ---, +++, && in the original file?

The first character of the line is reserved for the + and - character, so lines that start with those strings in the original files would appear as _--- _+++ and _&& where underscore is a space. The initial space character would cause the startsWith check to fail.

christopherfujino · 2022-05-17T18:12:19Z

packages/flutter_tools/lib/src/commands/migrate.dart

    required this.logger,
-    // TODO(garyq): Add each parameters in as subcommands land.
+    // TODO(garyq): Add each of these back in as they land.


add in what?

christopherfujino · 2022-05-17T18:16:08Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{};
+
+  /// Manually marks the lines in a diff that should be printed unformatted for visbility.
+  final Set<int> _initialDiffLines = <int>{0, 1};


Can you flesh out the dartdoc with this example?

christopherfujino · 2022-05-17T18:23:49Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+      'working-directory',
+      help: 'Specifies the custom migration working directory used to stage and edit proposed changes. '
+            'This path can be absolute or relative to the flutter project root.',
+      valueHelp: 'path',


Idk if this will be painful, but can we name this something else, maybe workspace-directory? I think this will be confusing with the current working directory of the tool invocation. In reading the code, I thought this field was a way to override the project directory.

christopherfujino · 2022-05-17T18:30:52Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+    );
+    argParser.addOption(
+      'project-directory',
+      help: 'The root directory of the flutter project.',


It looks like if this is not provided we default to the current working directory, can we document this?

christopherfujino · 2022-05-17T18:32:04Z

packages/flutter_tools/test/integration.shard/migrate_status_test.dart

+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/base/platform.dart';
+// import 'package:flutter_tools/src/base/terminal.dart';


Suggested change

// import 'package:flutter_tools/src/base/terminal.dart';

christopherfujino · 2022-05-17T18:40:06Z

packages/flutter_tools/test/integration.shard/migrate_status_test.dart

@@ -0,0 +1,202 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.


can you move this file to packages/flutter_tools/test/command.shard/permeable/migrate_status_test.dart?

christopherfujino · 2022-05-17T18:42:17Z

packages/flutter_tools/test/integration.shard/migrate_status_test.dart

+      ]
+    );
+    expect(logger.statusText, contains('''
+Newly addded file at added.file:


christopherfujino · 2022-05-17T18:43:29Z

packages/flutter_tools/test/integration.shard/migrate_status_test.dart

+deleted_files:
+''');
+
+    expect(appDir.childFile('lib/main.dart').existsSync(), true);


is this needed? we explicitly created it on line 55, are we testing it didn't get deleted?

Ahh, this is leftover from another test this was based on. I can remove.

christopherfujino · 2022-05-17T18:44:24Z

packages/flutter_tools/test/integration.shard/migrate_status_test.dart

+    conflictFile.createSync(recursive: true);
+    conflictFile.writeAsStringSync('''
+line1
+<<<<<<< /conflcit/conflict.file


christopherfujino · 2022-05-17T18:44:44Z

packages/flutter_tools/test/integration.shard/migrate_status_test.dart

+    expect(logger.statusText, contains('''
+@@ -1,3 +1,7 @@
+ line1
+<<<<<<< /conflcit/conflict.file


GaryQian · 2022-05-23T23:06:01Z

Ready for another round. Thanks! @christopherfujino

zanderso · 2022-06-01T15:59:05Z

packages/flutter_tools/lib/src/commands/migrate_status.dart

+    Directory workingDirectory = project.directory.childDirectory(kDefaultMigrateWorkingDirectoryName);
+    final String? customWorkingDirectoryPath = stringArg('working-directory');
+    if (customWorkingDirectoryPath != null) {
+      if (customWorkingDirectoryPath.startsWith(fileSystem.path.separator) || customWorkingDirectoryPath.startsWith(RegExp(r'[A-Z]:\\'))) {


You can get a path context from the filesystem and use https://pub.dev/documentation/path/latest/path/isAbsolute.html instead of re-implementing this.

Yep, already did it, just needed to sync it with the other PRs

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 28, 2022

GaryQian requested review from christopherfujino and blasten May 4, 2022 21:12

blasten reviewed May 5, 2022

View reviewed changes

GaryQian force-pushed the migratestatus branch from f1be828 to 8dfed47 Compare May 10, 2022 00:45

GaryQian requested a review from blasten May 10, 2022 05:52

GaryQian force-pushed the migratestatus branch from 75ffbf1 to 3fa7118 Compare May 11, 2022 18:24

GaryQian requested a review from zanderso May 12, 2022 22:57

christopherfujino reviewed May 17, 2022

View reviewed changes

GaryQian force-pushed the migratestatus branch from 4d1c8c1 to 5c34325 Compare May 19, 2022 00:29

GaryQian requested a review from christopherfujino May 20, 2022 21:54

GaryQian force-pushed the migratestatus branch from 988d7f0 to ef7c8ea Compare May 23, 2022 23:29

zanderso reviewed Jun 1, 2022

View reviewed changes

GaryQian requested a review from zanderso June 1, 2022 19:43

GaryQian added 8 commits June 1, 2022 13:58

Migrate status

02fa7f6

Add tests

da5a455

cleanup logging

ef9d52d

Merge with utils and manifest

ed63aa1

Fix analyze

e1f06a5

windows drive regex

e0ce03c

Fix regex

b0217a7

Fix null safety

57f9cf6

engine-flutter-autoroll mentioned this pull request Jun 5, 2022

Roll Flutter from 36fe51524f72 to 52c47e96a33d (182 revisions) flutter/packages#2205

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 5, 2022

0895130e5 Migrate status (flutter/flutter#102785)

44ad585

engine-flutter-autoroll mentioned this pull request Jun 5, 2022

Roll Flutter from 36fe51524f72 to 52c47e96a33d (182 revisions) flutter/packages#2206

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 5, 2022

0895130e5 Migrate status (flutter/flutter#102785)

d60c2fa

engine-flutter-autoroll mentioned this pull request Jun 5, 2022

Roll Flutter from 36fe51524f72 to 8294d9e91e39 (183 revisions) flutter/packages#2207

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 5, 2022

0895130e5 Migrate status (flutter/flutter#102785)

15f7b8f

engine-flutter-autoroll mentioned this pull request Jun 6, 2022

Roll Flutter from 36fe51524f72 to 1b2ee411aa13 (190 revisions) flutter/packages#2208

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022

0895130e5 Migrate status (flutter/flutter#102785)

f84cc17

engine-flutter-autoroll mentioned this pull request Jun 6, 2022

Roll Flutter from 36fe51524f72 to c181e450c8df (191 revisions) flutter/packages#2209

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022

0895130e5 Migrate status (flutter/flutter#102785)

49ce386

engine-flutter-autoroll mentioned this pull request Jun 6, 2022

Roll Flutter from 36fe51524f72 to 712752d2fb18 (192 revisions) flutter/packages#2211

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022

0895130e5 Migrate status (flutter/flutter#102785)

323413d

engine-flutter-autoroll mentioned this pull request Jun 6, 2022

Roll Flutter from 36fe51524f72 to 67a18250b17e (193 revisions) flutter/packages#2212

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022

0895130e5 Migrate status (flutter/flutter#102785)

9d89e45

engine-flutter-autoroll mentioned this pull request Jun 6, 2022

Roll Flutter from 36fe51524f72 to d5c62438e3e0 (194 revisions) flutter/packages#2213

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022

0895130e5 Migrate status (flutter/flutter#102785)

a0d16b5

engine-flutter-autoroll mentioned this pull request Jun 7, 2022

Roll Flutter from 36fe51524f72 to be0c1bd0f0ce (213 revisions) flutter/packages#2214

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022

0895130e5 Migrate status (flutter/flutter#102785)

b09bebd

engine-flutter-autoroll mentioned this pull request Jun 7, 2022

Roll Flutter from 36fe51524f72 to 78a0111c2093 (214 revisions) flutter/packages#2215

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022

0895130e5 Migrate status (flutter/flutter#102785)

3f43b3e

engine-flutter-autoroll mentioned this pull request Jun 7, 2022

Roll Flutter from 36fe51524f72 to 907529879f93 (215 revisions) flutter/packages#2216

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022

0895130e5 Migrate status (flutter/flutter#102785)

03c99e5

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022

0895130e5 Migrate status (flutter/flutter#102785)

096fde2

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022

0895130e5 Migrate status (flutter/flutter#102785)

e4676a0

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022

0895130e5 Migrate status (flutter/flutter#102785)

7b9cecb

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022

0895130e5 Migrate status (flutter/flutter#102785)

f5fa829

camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022

Migrate status (flutter#102785)

b4faa70

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022

0895130e5 Migrate status (flutter/flutter#102785)

46f3bef

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022

0895130 Migrate status (flutter/flutter#102785)

192f373

GaryQian mentioned this pull request Oct 21, 2022

[flutter_migrate] Reland apply, abandon, and status commands and add environment.dart flutter/packages#2723

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate status #102785

Migrate status #102785

GaryQian commented Apr 28, 2022 •

edited

Loading

flutter-dashboard bot commented Apr 28, 2022

blasten May 5, 2022

blasten May 5, 2022

GaryQian May 6, 2022

christopherfujino May 17, 2022

GaryQian Jun 2, 2022 •

edited

Loading

blasten May 5, 2022

GaryQian May 6, 2022

blasten May 5, 2022

GaryQian May 6, 2022

blasten May 5, 2022

GaryQian May 6, 2022

christopherfujino May 17, 2022

blasten May 5, 2022

GaryQian May 6, 2022

christopherfujino May 17, 2022

blasten May 5, 2022

GaryQian May 6, 2022 •

edited

Loading

christopherfujino May 17, 2022

christopherfujino May 17, 2022

christopherfujino May 17, 2022

christopherfujino May 17, 2022

christopherfujino May 17, 2022

christopherfujino May 17, 2022

christopherfujino May 17, 2022

christopherfujino May 17, 2022

GaryQian May 19, 2022

christopherfujino May 17, 2022

christopherfujino May 17, 2022

GaryQian commented May 23, 2022

zanderso Jun 1, 2022

GaryQian Jun 1, 2022

		@@ -0,0 +1,202 @@
		// Copyright 2014 The Flutter Authors. All rights reserved.

Migrate status #102785

Migrate status #102785

Conversation

GaryQian commented Apr 28, 2022 • edited Loading

flutter-dashboard bot commented Apr 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaryQian Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaryQian May 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaryQian commented May 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaryQian commented Apr 28, 2022 •

edited

Loading

GaryQian Jun 2, 2022 •

edited

Loading

GaryQian May 6, 2022 •

edited

Loading