Allow for escaped compiler and link optional parameters #33
Conversation
tools/common.hpp
Outdated
| std::string escape_transform(std::string input) { | ||
| for(auto i=input.begin(); i != input.end(); ++i) { | ||
| if(*i == '\\' && i+1 != input.end()) | ||
| i = input.erase(i); | ||
| } | ||
| return input; | ||
| } | ||
|
|
There was a problem hiding this comment.
How about erase-remove idiom?
| std::string escape_transform(std::string input) { | |
| for(auto i=input.begin(); i != input.end(); ++i) { | |
| if(*i == '\\' && i+1 != input.end()) | |
| i = input.erase(i); | |
| } | |
| return input; | |
| } | |
| std::string escape_transform(std::string input) { | |
| input.erase(std::remove(input.begin(), input.end(), '\\'), input.end()); | |
| return input; | |
| } | |
There was a problem hiding this comment.
erase-remove isn't quite the algorithm we want since it would convert the string "\-O2" to "-O2" instead of "-O2".
I added some docs in 52bce66 to address this.
There was a problem hiding this comment.
I don't think Michal's suggested code does same as original code.
Comment would be useful as I don't quite understand what this function is supposed to do
| @@ -163,7 +165,7 @@ namespace antler { | |||
| dep_subcommand->add_option("-t, tag", tag, "Tag associated with the dependency."); | |||
| dep_subcommand->add_option("-r, release", release, "Release version of the depedency."); | |||
| dep_subcommand->add_option("--digest, hash", hash, "Hash of the dependency."); | |||
|
|
|||
There was a problem hiding this comment.
general suggestion, please avoid whitespace only changes, it just messing history and makes changes more verbose. Not necessary to fix it for now as it is already there, so just for future.
There was a problem hiding this comment.
Sorry, I didn't actually notice I'd done it until AFTER Mikal had reviewed. Auto-updated in my editor. I generally agree, white space changes are to be avoided.
Added transforms that handle the escape (
\) character.Added regression tests to exercise these code paths.
Closes #27