Allow multiple targets on command line #160
Conversation
n-oden
force-pushed
the
multiple-argv
branch
from
0bd2748 to
b6af086
Compare
|
@incu6us I've updated this against current upstream/master and fixed the conflict |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #160 +/- ##
==========================================
+ Coverage 55.34% 57.00% +1.66%
==========================================
Files 11 11
Lines 1104 1177 +73
==========================================
+ Hits 611 671 +60
- Misses 470 482 +12
- Partials 23 24 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
Is the codecov test advisory, or are you asking for test coverage going forward? There's no current test set for main.go, but I suppose we could make one... |
main.go
Outdated
| printUsage() | ||
| os.Exit(1) |
There was a problem hiding this comment.
You are looping on argument to validate them.
Maybe you could report an error for each argument
Then after the loop printusage and exit, when at least an error is found
There was a problem hiding this comment.
Actually, given that validateRequiredParam is currently only checking to see if stdin exists in the cast of originPaths[0] being - I think this entire loop can be removed and the call to validateRequiredParam moved up into the previous block. @incu6us I'm going to make this change but let me know what you think?
This addresses incu6us#96 - iterate over the list of remaining arguments treating each as an origin path - fully remove support for the -filePath option - defer printing deprecation warnings until after all paths are processed - print deprecation warnings to stderr, not stdout
n-oden
force-pushed
the
multiple-argv
branch
from
06588ca to
70b0faf
Compare
|
@incu6us @ccoVeille addressed the comments, rebased on current upstream/master, and while I was at it spotted another issue: the call to helper.DetermineProjectName needed to be moved inside the loop over |
|
Great. Let's waif for @incu6us feedbacks then |
main.go
Outdated
| if err := validateRequiredParam(originPaths[0]); err != nil { | ||
| log.Printf("%s\n\n", err) | ||
| printUsage() | ||
| os.Exit(1) | ||
| } | ||
| } |
There was a problem hiding this comment.
Shouldn't be this?
| if err := validateRequiredParam(originPaths[0]); err != nil { | |
| log.Printf("%s\n\n", err) | |
| printUsage() | |
| os.Exit(1) | |
| } | |
| } | |
| } | |
| if err := validateRequiredParam(originPaths[0]); err != nil { | |
| log.Printf("%s\n\n", err) | |
| printUsage() | |
| os.Exit(1) | |
| } |
There was a problem hiding this comment.
I think I'm possibly misunderstanding something here. validateRequiredParam() will always return nil if the passed argument is anything other than reviser.StandardInput:
func validateRequiredParam(filePath string) error {
if filePath == reviser.StandardInput {
stat, _ := os.Stdin.Stat()
if stat.Mode()&os.ModeNamedPipe == 0 {
// no data on stdin
return errors.New("no data on stdin")
}
}
return nil
}So I think there's no point in calling it unless we know that we're operating on stdin, which would only happen if we find ourselves on the inside of if len(originPaths) == 0 || (len(originPaths) == 1 && originPaths[0] == "-") {
|
thanks @n-oden for your help. I have no any additional comments. agree with @ccoVeille about his comments to make some things more readable and simpler. |
| if err != nil { | ||
| log.Fatalf("%s", err) | ||
| } | ||
| os.Exit(0) |
There was a problem hiding this comment.
| os.Exit(0) | |
| os.Exit(1) |
There was a problem hiding this comment.
that was intentional -- if the error is non-nil, we'll exit with a non-zero exitval via log.Fatalf. If the error is nil, we print usage and exit successfully.
This may admittedly be overthinking it. :)
There was a problem hiding this comment.
Wow, indeed. Clever
A comment would be appreciated, saying ur will exit with exit 1
|
Thanks to all of you for getting my PRs merged and released! |
This addresses #96
iterate over the list of remaining arguments treating each as an origin path
defer printing deprecation warnings until after all paths are processed
print deprecation warnings to stderr, not stdout