Skip to content
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

builtins should not print entire usage summary on invalid use #3404

Closed
floam opened this issue Sep 23, 2016 · 8 comments
Closed

builtins should not print entire usage summary on invalid use #3404

floam opened this issue Sep 23, 2016 · 8 comments

Comments

@floam
Copy link
Member

@floam floam commented Sep 23, 2016

Consider

~ $ bind \e\c foo
Invalid token '\e\c'
fish: bind \e\c foo
           ^

This seems like it ought to be:

~ $ bind \e\c foo
fish: bind \e\c foo
           ^
Invalid token '\e\c'

Further:

~ $ functions -x
functions: Unknown option '-x'

       functions ‐‐ print or erase functions

   Synopsis
       functions [ ‐a | ‐‐all ] [ ‐n | ‐‐names ]
       functions ‐c OLDNAME NEWNAME
       functions ‐d DESCRIPTION FUNCTION
       functions [ ‐e | ‐q ] FUNCTIONS...

functions: Type 'help functions' for related documentation

We should place the error after the usage information. Sometimes it can be long and the error, far away.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Sep 24, 2016

We should place the error after the usage information.

No. The error should be foremost (i.e. "front and center"). That functions -x outputs usage text rather than a mention to run man functions is itself a bug IMHO.

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Sep 24, 2016

Agreed with @krader on both counts. Outputting the full usage spec just obscures the error message. I'd be in favor of replacing the "output full usage on error behavior" with a pointer to how to access the usage, e.g. to run man functions.

@floam
Copy link
Member Author

@floam floam commented Sep 24, 2016

That works too. The main issue is when the usage spec pushes the actual error off screen.

@zanchey
Copy link
Member

@zanchey zanchey commented Sep 30, 2016

That functions -x outputs usage text rather than a mention to run man functions is itself a bug IMHO.

Shall we track that here then?

@zanchey zanchey changed the title Error output seems backwards builtins should not print entire usage summary on invalid use Sep 30, 2016
@floam
Copy link
Member Author

@floam floam commented Oct 2, 2016

What might be the issue is not where we output usage output at all, but how "full" our manpage-derived usage output can be unnecessarily.

typical manpage-parsed output:
$ __fish_print_help fish_realpath

       fish_realpath ‐‐ Convert a path to an absolute path without symlinks

   Synopsis
       fish_realpath path

   Description
       This is an implementation of the external realpath command that
       doesn’t support any options. It’s meant to be used only by scripts
       which need to be portable. In general scripts shouldn’t invoke this
       directly. They should just use realpath which will fallback to this
       builtin if an external command cannot be found.

       If the path is invalid no translated path will be written to stdout
       and an error will be reported. This implementation behaves like the
       GNU command being invoked with ‐‐canonicalize‐existing.

should be more like 1-2 lines (and is where I did this by hand):

$ realpath
usage: realpath file
     resolves files as absolute paths without symlinks

@faho
Copy link
Member

@faho faho commented Mar 26, 2019

Yeah, I just removed the summary. It's fluff, people can use the man page or help.

@faho faho mentioned this issue Mar 26, 2019
10 tasks
@floam
Copy link
Member Author

@floam floam commented Apr 1, 2019

I sure wish we had a nice way to spit out a succinct usage line.

@floam
Copy link
Member Author

@floam floam commented Apr 1, 2019

I think we could have kept the man page thing and just more carefully cut up the output.

faho added a commit that referenced this issue Jun 25, 2019
Especially as, in this case, the documentation is quite massive.

Caught by porting string's test to littlecheck.

See #3404 - this was already supposed to be included.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants