Open
Conversation
- Heavily refactored arg parsing to use a simple search-and-dispatch pattern - The argument order no longer matters. The last filename entered is the one used. - longopts can now take arguments in --key=value or --key value format - Added -c to --color - -h/--help is now a canon option - -- now disables opt parsing instead of merely exiting the arg parsing loop - updated man page to explain what -s/--sector does - made an effort to keep code style as similar to pixel's as possible (next line brace, 2-space indent) Feel free to review my changes. I'm not used to automake/m4, but I think I got everything. I even logged my changes in the changelog, which I've duplicated above (that's just how I format my commits). I made an effort to keep the style as similar to yours as possible. I observed camelCase, 2-space indent and next line braces. I did break the rules in a couple places, namely parseArg_matchOpt. The underscores are a "pseudo namespace". I think it's reasonable. I also added one type, ParseArgCallbackFn, in PascalCase because I assume that's the convention (PascalCase for types, camelCase for functions and variables). I tried to keep the changes small in scope, which is why the LUT in parseArg is an anonymous struct and why there aren't any enums, like for the return value of parseArg_matchOpt. The new arg parser loop doesn't behave completely identically to the old one, but the changes I made are sane and reasonable. For example, the order no longer matters, you can do `filename -s` or `-s filename`. There was also some stipulation about throwing the usage if argc > 1 after the arg parser loop is done. I did away with that. I was careful to preserve the behavior when fileName is NULL: the program will make one attempt to obtain it interactively (findFile), and then either exit with an error message or open the file. That's about all that comes to mind. Nice app! -Braden Best (bradenbest)
Author
|
Oh, I forgot to include this in the commit: fixes #70 |
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Feel free to review my changes. I'm not used to automake/m4, but I think I got everything. I even logged my changes in the changelog, which I've duplicated above (that's just how I format my commits).
I made an effort to keep the style as similar to yours as possible. I observed camelCase, 2-space indent and next line braces. I did break the rules in a couple places, namely parseArg_matchOpt. The underscores are a "pseudo namespace". I think it's reasonable. I also added one type, ParseArgCallbackFn, in PascalCase because I assume that's the convention (PascalCase for types, camelCase for functions and variables).
I tried to keep the changes small in scope, which is why the LUT in parseArg is an anonymous struct and why there aren't any enums, like for the return value of parseArg_matchOpt.
The new arg parser loop doesn't behave completely identically to the old one, but the changes I made are sane and reasonable. For example, the order no longer matters, you can do
filename -sor-s filename. There was also some stipulation about throwing the usage if argc > 1 after the arg parser loop is done. I did away with that. I was careful to preserve the behavior when fileName is NULL: the program will make one attempt to obtain it interactively (findFile), and then either exit with an error message or open the file.That's about all that comes to mind. Nice app!
-Braden Best (bradenbest)
edit: fixes #70