Add support for locations starting with https://github.com/
#47
Conversation
Get rid of `gh` error if not installed
| @@ -86,6 +86,7 @@ namespace antler::system { | |||
| for (const auto& arg : args) { | |||
| cmd += " " + arg; | |||
| } | |||
| cmd += " 2>&1"; | |||
There was a problem hiding this comment.
what does it affect? if there will be error you return nullopt and if not you'll unlikely to have error output at all. And even if you do, parsing of output will likely break.
There was a problem hiding this comment.
It resolves issue: sh: 1: gh: not found in the original issue.
I believe Bucky's intention was to run command line gh --version quietly (the function name is execute_quiet).
While stdout was silenced, the stderr was still printing error (which was ignored anyway).
There was a problem hiding this comment.
got you. in that case cmd += "2> /dev/null" sounds to me like more self-descriptive
There was a problem hiding this comment.
In current usages of execute_quiet both 2>&1 and 2> /dev/null are fine.
If someone would like to read the stderr if the return code is 0 (there are such cases) with the existing solution he will have such chance. In yours we are losing stderr output. So I will leave the current solution.
There was a problem hiding this comment.
I doubt that could be a case. if we will have non-critical error output on zero return code that may corrupt parsing of a valid data. Ideally that should be similar to subprocess.run in python. i.e. stdout and stderr piping is customizable. but I would leave it to next issue if that will be really needed. If you believe your case is more likely to happen than mine, just leave comment like pipe error output from console to stdout so caller can parse error string and I'll approve
Additionally get rid of
gherror if not installed.Fix an update of the existing dependency: previously update was ignored (because executing
emplaceonunordered_mapis ignored ifkeyalready exists).When it comes to supporting
https://github.com: now dependencies are still stored as shorthands (i.e.org/project), so I'm normalizing the location before validating / adding location to the dependency.