charlesdaniels / bitshuffle

BSD 3-Clause "New" or "Revised" License
5 stars 0 forks source link

Infer encode/decode (major changes) #26

Closed jyn514 closed 6 years ago

jyn514 commented 6 years ago

Assign defaults after inferring argument

Quit if no arguments given Many layers of checks to infer arguments Decisions made as follows: (assuming encode/decode not passed)

Other refactoring: find_editor() is its own function (simplifies logic by a great deal)

bx-r0 commented 6 years ago

Does "Quit if no arguments given" prevent the use of the tool being used with piping?

jyn514 commented 6 years ago

Only if no arguments are given on the command line. This behavior is specified by https://github.com/charlesdaniels/bitshuffle/issues/1. The tools runs correctly if arguments are specified and input is piped.

jyn514 commented 6 years ago

You make a good point though- if it's piped, the user won't be looking for help, so we should consider inferring an argument.

Also, I want to refactor the control structure, it has all the tabs because of design decision I never commited

charlesdaniels commented 6 years ago

no output: -> encode (why?)

I don't remember specifying this... I can't think of a justification for it; unless the user also specified --input, in which case it makes sense.

no input and interactive -> decode (why?)

This only makes sense if standard in is a tty; it means the use is running interactively, and thus that they want their $VISUAL to open so they can paste data in.

input and not interactive -> quit with error

Huh? This would imply the user is using it with a pipeline. For example bitshuffle --input /some/file | some other script. One specific use case for this functionality would be where the input needs to come from a named pipe (although the user could easily direct it to stdin.

input and output -> quit with error

This is not an error condition, it implies that encoded packets are to be read from the input file specified and written to the specified output file.

find_editor() is its own function (simplifies logic by a great deal)

This was a good idea. I tried to find a library to do this, but could not.

You make a good point though- if it's piped, the user won't be looking for help, so we should consider inferring an argument.

Just because stdin is not a tty does not mean bitshuffle is running in a pipeline (although it is a strong indicator for the purpose of heuristics). Also, the help message is printed on stderr (as far as I know) which would not be captured by the pipe without the user specifically telling the pipe to capture it.

jyn514 commented 6 years ago

To clarify, are these the changes you propose to the pull request?

input and no output -> encode input and output -> decode

As an aside: We should probably find a way to describe input/output as required in the help message, it's very uninformative otherwise.

charlesdaniels commented 6 years ago

To clarify, are these the changes you propose to the pull request? input and no output -> encode input and output -> decode

Yes, that is correct.

As an aside: We should probably find a way to describe input/output as required in the help message, it's very uninformative otherwise.

Yes, I'm planning to re-work some of the verbiage around this project at some point, there are a number of terms which are used in inconsistent or unclear ways.

jyn514 commented 6 years ago

Merged this into dev; too many changes for it to go straight to master.

charlesdaniels commented 6 years ago

All of the inference considerations that we discussed have been implemented as of c4a0290

Note that I decided that having --input AND --output simultaneously should not imply anything special.