fail safely on teletype add if the current directory is not a teletype app
#72
Conversation
| @@ -73,7 +75,7 @@ def execute(input: $stdin, output: $stdout) | |||
| cmd_integ_test_file = "#{test_dir}/integration/#{cmd_name_path}_#{test_dir}.rb" | |||
| cmd_unit_test_file = "#{test_dir}/unit/#{cmd_name_path}_#{test_dir}.rb" | |||
|
|
|||
| if !subcmd_present? | |||
| unless subcmd_present? | |||
There was a problem hiding this comment.
Style/UnlessElse: Do not use unless with else. Rewrite these with the positive case first.
There was a problem hiding this comment.
I dunno, I wholeheartedly disagree with this. I feel like the reason unless exists is to not have if !... ... @piotrmurach, what's your take?
This isn't directly related to this change, it was just a thing I modified while running though the file - I'll revert it if needbe.
There was a problem hiding this comment.
I don't have a strong preference either way. This change is fine by me.
| @@ -73,7 +75,7 @@ def execute(input: $stdin, output: $stdout) | |||
| cmd_integ_test_file = "#{test_dir}/integration/#{cmd_name_path}_#{test_dir}.rb" | |||
| cmd_unit_test_file = "#{test_dir}/unit/#{cmd_name_path}_#{test_dir}.rb" | |||
|
|
|||
| if !subcmd_present? | |||
| unless subcmd_present? | |||
There was a problem hiding this comment.
I don't have a strong preference either way. This change is fine by me.
| def validate_pwd | ||
| fail ::TTY::CLI::Error, @pastel.red("This doesn't look like a teletype app directory - are you in the right place?") unless | ||
| options[:force] || (@app_path + "lib/#{namespaced_path}").exist? | ||
| end |
There was a problem hiding this comment.
Since we add commands to lib/app/commands folder, maybe we should check that this location is present? After displaying the error message, we could potentially advise on how to run the command with an example or similar to Rails automatically run teletype add --help ?
There was a problem hiding this comment.
I originally was checking that, yes - but it was causing other tests to fail (they set up a fixture that did not make a 'commands' dir). The add command creates the 'commands' directory if it doesn't exist (seemingly):
[katelyn@h test]$ rm -r lib/test/commands
[katelyn@h test]$ teletype add haha
create spec/integration/haha_spec.rb
create spec/unit/haha_spec.rb
create lib/test/commands/haha.rb
create lib/test/templates/haha/.gitkeep
inject lib/test/cli.rb
[katelyn@h test]$ ls -l lib/test/commands
total 4
-rw-r--r-- 1 katelyn katelyn 332 Sep 28 15:07 haha.rb
..so I don't think the check is necessary.
I like the idea of outputting the 'help' - yeah. I'll add that!
|
Thank you! ❤️ My plan is to cut a new release this month. |
Describe the change
Adds a
validate_pwdmethod to the "add" command that makes sure that a gemspec & directory exists before trying to create the command.Why are we doing this?
A friendly message is better than a stack trace.
Benefits
This will improve user understanding when accidentally trying to run
teletype addoutside of the project directory.Should close #62
Drawbacks
There's no way to directly check that the pwd is actually a teletype app, vs another app that happens to have the same structure. However, that was the case before too - this just covers one edge case.
Requirements
Put an X between brackets on each line if you have done the item:
[...] Tests written & passing locally?
[x] Code style checked?
[x] Rebased with
masterbranch?[x] Documentaion updated?