-
Notifications
You must be signed in to change notification settings - Fork 80
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
CLI arguments are inconsistent #243
Comments
Can you show us your ideal I think the We can decide to reserve I don't have opinions on Regarding
You would keep the option and hide it from the --help section ? Or just hide it from the general documentation (README, wiki) ? In terms of options both |
Interestingly, binwalk's use of
I don't think that we need a short name for that
I'd probably rename it to |
If there is a general consensus that we should rework the switches I am willing to design it :) At this stage it is more of a question to see if you are agreeing with my assessment. |
Maybe we should also add a --version argument |
Updated OP with suggested changes in help, also containing a potential future direction to add extensible extract configuration options without cluttering the core UX |
This flag is available with the latest version (23.8.11). |
Now almost all command line arguments have a short and a long name. Most short names are undecipherable or misleading without reading through the help. Also, few argument names leak implementation details. We shouldn't expose short switches for more obscure functionality at all and the ones we are exposing should be as intuitive as possible.
I think we should double think about the names of these switches before 1.0:
-n
: It means dry-run in most command line programs and it may make sense to have a dry-run switch in the future, but right now it is configuring entropy calculation.-e
: Output location can be configured either via positional argument or-o
many times. As we are supporting arbitrary input files, we shouldn't use a positional argument though.--process-num
/-p
: There is no generally established name here. Some times-j
is used to denote "jobs", or-p
to indicate parallelism. I think we shouldn't indicate on our public API that we are using processes for parallelism. (It is also true for the actualprocess_file
function too)--show-external-dependencies
this is an odd one we agreed about in the past that it doesn't really seem right and may make sense to use a subcommand instead but we didn't want to introduce them just for this functionality. If we are to use subcommands elsewhere, we should reconsider this as wellSuggested help output (for existing functionality)
Suggested help format containing potential future options
Potential future direction with subcommands
Currently we have only the default implicit
extract
and the somewhat clunkyshow-external-dependencies
functionality which doesn't warrant the addition of subcommands. If we want to add in the future e.g. Forcing a given extractor to a file, it could make sense to add subcommands (I know, I know, unpack and extract are awful verbs to use together...)The text was updated successfully, but these errors were encountered: