-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add long options like --part #1936
base: main
Are you sure you want to change the base?
Conversation
Switch the command line argument parser from getopt(3) to getopt_long(3) and add a few long options as aliases to existing short options, e.g. "--help" and "--part" being aliases for "-?" and "-p", respectively. The getopt_long(3) function is available on GNU, BSD, and the existing msvc/getopt.[ch] already implements getopt_long() on Windows. This should cover all systems avrdude supports. Adapt the avrdude usage message shown by "-?" or "--help" to show the new long options. TODO: Adapt man page and texi manual reflecting the long options. Closes: avrdudes#1922
c012638
to
818e0e8
Compare
src/main.c
Outdated
{"quiet", no_argument, NULL, 'q'}, | ||
{"reconnect", no_argument, NULL, 'r'}, | ||
{"terminal", no_argument, NULL, 't'}, | ||
{"tty", no_argument, NULL, 't'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I look at the list carefully, I don't think tty
is a good long option name to signify -t
for the principal reason that tty describes a communication method (teletype), which is orthogonal to what -t
does. Terminal input can come from a file, from a pipe, via a USB connection etc and not necessarily from a tty. I'd prefer to either have no second long option for -t
, but if there is an appetite for a second option, I'd much rather have one of --interactive
, --console
, --avr-console
, --shell
or similar. We normally use the wording terminal or interactive terminal in the documentation for -t
, so --terminal
might be sufficient.
Could we add --terminal-command
for -T
(note capital T), which like -U
is not interactive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"shell" implies turing completeness. "interactive" captures the spirit well.
{"noerase", no_argument, NULL, 'D'}, | ||
{"erase", no_argument, NULL, 'e'}, | ||
{"logfile", required_argument, NULL, 'l'}, | ||
{"test", no_argument, NULL, 'n'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-n
meant originally don't write to the AVR part for -U
operations, where -n
would write the file to stdout instead of the AVR memory. When other ways to write to the AVR were implemented (-t
, -T
, ...) there was no way to test these in a dryrun as it were until the -c
programmer dryrun
was implemented. I now wonder whether --test
should be --test-U
or rather --test-update
to clarify the role of that option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think --test
is the wrong mnemonic. Given we have --memory
for -U
we should rename --test
to --test-memory
as its scope is confined to -U/--memory
operations.
Cool improvement! I like this PR |
I'm happy with the PR, but I have a few suggestions, based on my personal opinions:
|
I wonder whether A set of I guess |
Good idea! But how would we let the user specify the format ( :i, :r, :m etc.)?
|
There are other characters which can be used as separators. Or With long options, we are not constrained by the idiosyncrasies of the past. |
Correct. Avrdude knows three operations, some 48 memories and 12 file formats. Is the suggestion for main.c to entertain some 144 long options for all (op, memory) combos? Or 1728 long options to expand the space to all (op, memory, file format) combos? There is the added difficulty that a new part in a custom config file may well introduce a new memory, which requires new long options to automatically spring up in that case.
Projecting the And how should the command line express
That's another pitfall: Also note that allowing a different way than appending All of above can be done and done well, but should it? I believe the best thing for now is to have a one-to-one long replacement option for |
I think we should wrap it up and merge this PR. |
Note to self: The usage message should mention e.g. --part and possibly other long options. |
It would be great to wrap up this PR. I support all of @MCUdude's suggestions here. Suggesting a new syntax for Over to you, @ndim to continue the great work here! |
+ remove duplicates
@ndim sorry for pushing directly to your branch. I was trying to push to my own fork or your development branch, but I messed up somewhere on the way. If you look at the last commit, what I did was to add a few missing long opts to the help text, and remove duplicate log opts. |
I think the implementation itself is OK now. I've done the manpage documentation but still have the texinfo docs left. |
@stefanrueger I think this PR is mostly done now, so please review! The only longopt name I'm not so sure about is @ndim sorry for the noise. The reason why I messed up/couldn't fork your branch is because I already have a fork of the Avrdude project. And I misunderstood and accidentally pushed straight to your fork instead of to a "fork of your fork". But at least now the docs should be OK, and the missing longest names in the help text is now present. |
Switch from getopt(3) to getopt_long(3) and add a few long options as aliases to existing short options, e.g. "--help" and "--part" being aliases for "-?" and "-p", respectively.
The getopt_long(3) function is available on GNU, BSD, and the existing msvc/getopt.[ch] already implements getopt_long() on Windows. This should cover all systems avrdude supports.
getopt_long(3)
instead ofgetopt(3)
--config
--part
and--port
#1922 (comment)Closes: #1922