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

feat: allow very_good create . #996

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 28 additions & 3 deletions lib/src/commands/create/commands/create_subcommand.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,24 @@ abstract class CreateSubCommand extends Command<int> {
return Directory(directory);
}

/// Gets the project name.
String get projectName {
/// The project name that the user provided.
///
/// Since the project name could be user provided as either:
/// 1. A valid package name
/// 2. '.', the current working directory (the project assumes the cwd's name)
/// this needs to exist to provide a fallback value for [outputDirectory] and
/// [projectName].
String get _projectName {
final args = argResults.rest;
_validateProjectName(args);
return args.first;
}

/// Gets the project name.
String get projectName => _projectName == '.'
? path.basename(Directory.current.path)
: _projectName;

Comment on lines +124 to +141
Copy link
Member

Choose a reason for hiding this comment

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

I think we should rework this. Introducing . as a projectName and using it to find the name of the directory is kinda confusing because what if I pass path/to/my/folder instead?

. implies current directory which implicitly also means we can pass any path, which isnt the case. However this is the case for flutter create so maybe we should just make it that we can pass any given path and use the last part of that path as the projectName and create the template at the given path.

What you think?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @wolfenrain, thanks for the positive feedback.

I agree, let's do it the same way as the Flutter tool.

This means we'd remove --output-directory as an argument.

As far as very_good create <arg>, here are some requirements that I could implement:

  • Allow relative paths as arg.
  • Allow absolute paths as arg.
  • Use the last path segment of arg as the project name.
  • Remove the --output-directory argument.

Let me know if you'd like me to proceed. Thanks for catching my oversight 👍

Copy link
Member

Choose a reason for hiding this comment

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

I do think that is a good approach, so arg would be any type of path indeed, it would be nice if we can keep backwards compatibility with the --output-directory. Maybe by just checking if it is supplied, if it is show that it is deprecated and just take that over whatever was in arg, assuming arg was a path?

PS: it would be very_good create <template> <output directory>

Copy link
Member

Choose a reason for hiding this comment

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

Hey @a-wallen just following up here to see if you are planning on tackling this still?

/// Gets the description for the project.
String get projectDescription => argResults['description'] as String? ?? '';

Expand Down Expand Up @@ -159,6 +170,15 @@ abstract class CreateSubCommand extends Command<int> {
}

final name = args.first;

if (name == '.') {
logger.detail(
'Since the project name is ".", the current directory will'
' be used as the project name.',
);
return;
}

final isValidProjectName = _isValidPackageName(name);
if (!isValidProjectName) {
usageException(
Expand Down Expand Up @@ -211,7 +231,12 @@ abstract class CreateSubCommand extends Command<int> {

await template.onGenerateComplete(
logger,
Directory(path.join(target.dir.path, projectName)),
Directory(
[
target.dir.path,
if (_projectName != '.') projectName,
].join(Platform.pathSeparator),
),
);

return ExitCode.success.code;
Expand Down
57 changes: 56 additions & 1 deletion test/src/commands/create/create_subcommand_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:args/args.dart';
import 'package:args/command_runner.dart';
import 'package:mason/mason.dart';
import 'package:mocktail/mocktail.dart';
import 'package:path/path.dart' as path;
import 'package:test/test.dart';
import 'package:very_good_cli/src/commands/create/commands/create_subcommand.dart';
import 'package:very_good_cli/src/commands/create/templates/template.dart';
Expand Down Expand Up @@ -285,7 +286,61 @@ Run "runner help" to see global options.''';
that: isA<Directory>().having(
(d) => d.path,
'path',
'test_dir/test_project',
path.join('test_dir', 'test_project'),
),
),
),
).called(1);
});

test('allows projects to be cwd (.)', () async {
final expectedProjectName = path.basename(Directory.current.path);

final result = await runner.run([
'create_subcommand',
'.',
]);

expect(result, equals(ExitCode.success.code));
verify(() => logger.progress('Bootstrapping')).called(1);

verify(
() => hooks.preGen(
vars: <String, dynamic>{
'project_name': expectedProjectName,
'description': 'A Very Good Project created by Very Good CLI.',
},
onVarsChanged: any(named: 'onVarsChanged'),
),
);
verify(
() => generator.generate(
any(
that: isA<DirectoryGeneratorTarget>().having(
(g) => g.dir.path,
'dir',
'.',
),
),
vars: <String, dynamic>{
'project_name': expectedProjectName,
'description': 'A Very Good Project created by Very Good CLI.',
},
logger: logger,
),
).called(1);
expect(
progressLogs,
equals(['Generated ${generatedFiles.length} file(s)']),
);
verify(
() => template.onGenerateComplete(
logger,
any(
that: isA<Directory>().having(
(d) => d.path,
'path',
'.',
),
),
),
Expand Down