search

TritonDataCenter / node-manta

Node.js SDK for Manta
75 stars 54 forks source link

mget -H option documented incorrectly #313

Closed kellymclaughlin closed 1 year ago

kellymclaughlin commented 7 years ago

The documented behavior of the -H option to mget does not match the actual behavior. The documentation indicates this option should print the HTTP headers to stderr and implies the option should not accept any arguments. In practice the option does expect an argument in the same way that the -H option to mput behaves.

kellymclaughlin commented 7 years ago

Perhaps the best solution here is to leave the current behavior of the -H option for mget intact to maintain consistency with the other m* commands and add a different options for displaying the headers. It could be useful to pass along headers for cache control or other purposes with an mget request.

Any thoughts?

trentm commented 7 years ago

@kellymclaughlin Agree on leaving -H,--header behaviour as is and using a new option to dump the headers.

To clarify it is the man page (man mget) that is incorrect. The mget -h output correctly documents the supported options. FWIW, the man page also has a number of other problems:

For comparison curl has: 

       -i, --include
              (HTTP) Include the HTTP-header in the  output.  The  HTTP-header  includes
              things like server-name, date of the document, HTTP-version and more...

I'm not sure if the intent of this mget option is to dump headers in that format. -i is already taken and --include is a little ambiguous. Perhaps: --include-headers. Tab completion could make that reasonable to "type".

kellymclaughlin commented 7 years ago

@trentm --include-headers seems like a good option so maybe we can go with that. I can also make updating and correcting the man page part of this task.

bahamat commented 5 years ago

Can we, as a first step just fix the man page?

bahamat commented 5 years ago

This CR fixes the man page: https://cr.joyent.us/5761

trentm commented 5 years ago

stmaped, LGTM (the man page update I mean)

bahamat commented 5 years ago

Created #364 for the new behavior request.

bahamat commented 5 years ago

Fixed in 76545399b0e0e080c37f5af6cc1629fd3a2b7ffc.

davepacheco commented 5 years ago

Thanks for fixing this. I just noticed it looks like the new manual page description of -H doesn't indicate that it takes an argument.

bahamat commented 5 years ago

@davepacheco You're right. Not only that, but mget doesn't accept --headers, only --header.

New CR: https://cr.joyent.us/5773

bahamat commented 1 year ago

Subsequent fix in 04bd357.