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.

Sign up for GitHub

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

Fix generated_plugin_registrant.dart not being ignored #52

Merged
merged 4 commits into from
May 30, 2021
Merged

Fix generated_plugin_registrant.dart not being ignored #52

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d42e51
fix generated_plugin_registrant not being ignored
bartekpacia May 10, 2021
d4d88f0
ignore IDE files
bartekpacia May 26, 2021
a42bc6d
Merge branch 'master' into fix_generated_plugin_registrant
gleich May 26, 2021
2e30e6c
fix(lint): dartfmt main.dart
gleich May 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"dart.lineLength": 80
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually see a comment I made later please

20 changes: 16 additions & 4 deletions bin/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,17 @@ void main(List<String> args) {

// Getting all the dart files for the project
final dartFiles = files.dartFiles(currentPath, args);
if (dependencies.contains('flutter/') &&
dartFiles
.containsKey('${currentPath}/lib/generated_plugin_registrant.dart')) {
final containsFlutter = dependencies.contains('flutter');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR could basically be just changing flutter/ to flutter, but I decided to improve some other parts a bit as well.

final containsRegistrant = dartFiles
.containsKey('${currentPath}/lib/generated_plugin_registrant.dart');

stdout.writeln('contains flutter: ${containsFlutter}');
stdout.writeln('contains registrant: ${containsRegistrant}');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to remove this after merge, I added this so it's more clear.


if (containsFlutter && containsRegistrant) {
dartFiles.remove('${currentPath}/lib/generated_plugin_registrant.dart');
}

for (final pattern in ignored_files) {
dartFiles.removeWhere((key, _) =>
RegExp(pattern).hasMatch(key.replaceFirst(currentPath, '')));
Expand All @@ -90,7 +96,13 @@ void main(List<String> args) {
}

final sortedFile = sort.sortImports(
file.readAsLinesSync(), packageName, emojis, exitOnChange, noComments);
file.readAsLinesSync(),
packageName,
emojis,
exitOnChange,
noComments,
filePath: filePath,
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this from that vscode settings file? If so feel free to keep the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's me – I almost always (when it makes sense) add the trailing comma because it makes the code prettier :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove the whole .vscode directory and add it to .gitignore, but then it'd be nice to include some info in README.md or CONTRIBUTING.md that this project uses lineLength = 80

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great, thank you!

filesFormatted++;

dartFiles[filePath]?.writeAsStringSync(sortedFile.sortedFile);
Expand Down
14 changes: 7 additions & 7 deletions lib/sort.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// 🎯 Dart imports:
import 'dart:io';

// 📦 Package imports:
import 'package:tint/tint.dart';

/// Sort the imports
/// Returns the sorted file as a string at
/// index 0 and the number of sorted imports
Expand All @@ -13,8 +10,9 @@ ImportSortData sortImports(
String package_name,
bool emojis,
bool exitIfChanged,
bool noComments,
) {
bool noComments, {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional named argument so tests don't require refactors.

String? filePath,
}) {
String dartImportComment(bool emojis) =>
'//${emojis ? ' 🎯 ' : ' '}Dart imports:';
String flutterImportComment(bool emojis) =>
Expand Down Expand Up @@ -156,8 +154,10 @@ ImportSortData sortImports(
final sortedFile = sortedLines.join('\n');
final original = lines.join('\n') + '\n';
if (exitIfChanged && original != sortedFile) {
stdout.write('\n┗━━🚨 ');
stdout.write('Please run import sorter!'.bold().red());
if (filePath != null) {
stdout.writeln(
'\n┗━━🚨 File ${filePath} does not have its imports sorted.');
Copy link
Contributor Author

@bartekpacia bartekpacia May 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed .bold().red() - believe me or not, but it doesn't show up on Ubuntu 20.04 LTS machines.

Also, printing the file name on which the tool failed is very useful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please bring back the red and bold display. It's most likely a problem with your terminal's display colors/system if the color isn't being displayed correctly. It works fine for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't customize anything - just a default Ubuntu terminal (test it out for yourself – it's available on GitHub Actions). I think that many people are using Ubuntu and may encounter this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we can cut it then

}
exit(1);
}
if (original == sortedFile) {
Expand Down