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

Builtin wrapper functions produce a backtrace with invalid options #5434

Closed
meain opened this issue Dec 27, 2018 · 9 comments
Closed

Builtin wrapper functions produce a backtrace with invalid options #5434

meain opened this issue Dec 27, 2018 · 9 comments

Comments

@meain
Copy link

@meain meain commented Dec 27, 2018

fish version: 2.7.1

If I pass in an unknown argument to a builtin, it produces some unnecessary output and not just the Unknown option message.

meain@plank ~> cd -r
cd: Unknown option '-r'
/usr/local/Cellar/fish/2.7.1/share/fish/functions/cd.fish (line 30):
    builtin cd $argv
    ^
in function 'cd'
        called on standard input
        with parameter list '-r'


       cdcd - change directory
        -

   Synopsis
       cd [DIRECTORY]

cd: Type 'help cd' for related documentation
@faho
Copy link
Member

@faho faho commented Dec 27, 2018

Duplicate of #3404

@faho faho marked this as a duplicate of #3404 Dec 27, 2018
@faho faho closed this Dec 27, 2018
@faho faho added the duplicate label Dec 27, 2018
@floam
Copy link
Member

@floam floam commented Jan 21, 2019

I don't think he's complaining about seeing the synopsis, but that he's getting this part:

/usr/local/Cellar/fish/2.7.1/share/fish/functions/cd.fish (line 30):
    builtin cd $argv
    ^
in function 'cd'
        called on standard input
        with parameter list '-r'

@meain is that correct?

@meain
Copy link
Author

@meain meain commented Jan 21, 2019

@floam yes

@floam floam reopened this Jan 21, 2019
@floam floam removed the duplicate label Jan 21, 2019
@zanchey zanchey marked this as not a duplicate of #3404 Jan 22, 2019
@zanchey zanchey changed the title Extra output on unknown argument to builtins Builtin wrapper functions produce a backtrace with invalid options Jan 22, 2019
@zanchey zanchey added this to the fish-future milestone Jan 22, 2019
@zanchey
Copy link
Member

@zanchey zanchey commented Jan 22, 2019

That could be fixed, though it's low-impact.

@zanchey zanchey added the bug label Jan 22, 2019
@faho
Copy link
Member

@faho faho commented Mar 26, 2019

Okay, what should we change here?

We could completely remove the complete backtrace and help, but that would just leave:

$ cd -r
cd: Unknown option '-r'

which seems alright if it's directly on the commandline like in that example, but what if it's inside a script somewhere? Then you can play hunt-the-bug.

We could probably remove the help or just point to it with "see help cd for help".

We should also shorten the backtrace, because this is ridiculous:

Screenshot_20190326_155705

How about removing the multiple "on standard input" lines and the multiple empty lines here, for one?

So the backtrace looks more like

cd: Unbekannte Option '-r'
/usr/share/fish/functions/cd.fish (Zeile 40):
    builtin cd $argv
    ^
in function 'cd'
        with parameter list '-r'
in function 'd'
in function 'b'
in function 'a'
        called on standard input

?

Line information for all the functions would actually be nice, though.

I'm relabelling this as an enhancement because this is currently how it is supposed to behave.

@faho faho added enhancement and removed bug labels Mar 26, 2019
faho added a commit that referenced this issue Mar 26, 2019
This printed things like

```
in function 'f'
        called on standard input

in function 'd'
        called on standard input

in function 'b'
        called on standard input

in function 'a'
        called on standard input

```

As a first step, it removes the empty lines so it's now

```
in function 'f'
        called on standard input
in function 'd'
        called on standard input
in function 'b'
        called on standard input
in function 'a'
        called on standard input
```

See #5434.
@faho
Copy link
Member

@faho faho commented Mar 26, 2019

Okay, I've removed the empty lines.

What I'm working on now:

  • Remove the "called on standard input" message

  • Print the function arguments on the same line as the function

  • Stop calling it "parameter list"

faho added a commit to faho/fish-shell that referenced this issue Mar 26, 2019
Now:

```
cd: Unknown option '-r'
~/dev/fish-shell/share/functions/cd.fish (line 40):
    builtin cd $argv
    ^
in function 'cd' with arguments '-r'
in function 'f'
in function 'd'
in function 'b' with arguments '-1q --wurst'
in function 'a'
	called on standard input
```

See fish-shell#5434.
faho added a commit to faho/fish-shell that referenced this issue Mar 26, 2019
This was printed basically everywhere.

The user knows what they executed on standard input.

A good example:

```fish
set c (subme 513)
```

used to print

```
fish: Too much data emitted by command substitution so it was discarded

    set -l x (string repeat -n $argv x)
             ^
in function 'subme'
	called on standard input
	with parameter list '513'
in command substitution
	called on standard input
```

and now it is

```
fish: Too much data emitted by command substitution so it was discarded

    set -l x (string repeat -n $argv x)
             ^
in function 'subme' with arguments '513'
in command substitution
```

See fish-shell#5434.
faho added a commit to faho/fish-shell that referenced this issue Mar 26, 2019
@faho
Copy link
Member

@faho faho commented Mar 26, 2019

Okay, here's what I've got:

cd: Unknown option '-r'
~/dev/fish-shell/share/functions/cd.fish (line 40): 
    builtin cd $argv
    ^
in function 'cd' with arguments '-r'
in function 'f'
in function 'd'
in function 'b' with arguments '-1q --wurst \$var ""'
in function 'a'

from

cd: Unbekannte Option '-r'
/usr/share/fish/functions/cd.fish (Zeile 40): 
    builtin cd $argv
    ^
in function 'cd'
	called on standard input
	with parameter list '-r'

in function 'f'
	called on standard input

in function 'd'
	called on standard input

in function 'b'
	called on standard input
	with parameter list '-1q --wurst $var '

in function 'a'
	called on standard input

(plus the docs in both - also the missing translation is down to me not yet installing the new version)

Any more ideas?

@faho faho removed this from the fish-future milestone Mar 26, 2019
@faho faho added this to the fish 3.1.0 milestone Mar 26, 2019
@faho
Copy link
Member

@faho faho commented Mar 26, 2019

The example from the OP:

cd: Unknown option '-r'
~/dev/fish-shell/share/functions/cd.fish (line 40): 
    builtin cd $argv
    ^
in function 'cd' with arguments '-r'

       cd - change directory

       cd [DIRECTORY]

       cd changes the current working directory.

cd: Type 'help cd' for related documentation

faho added a commit that referenced this issue Mar 26, 2019
This now displays

- the error message

- a (significantly shorter) backtrace

- A call to open `help $cmd` if necessary

See #5434.
Fixes #3404.
@faho
Copy link
Member

@faho faho commented Mar 26, 2019

Okay, now after #3404, the example looks like

$ cd -r
cd: Unknown option '-r'

~/dev/fish-shell/share/functions/cd.fish (line 40): 
    builtin cd $argv
    ^
in function 'cd' with arguments '-r'

(Type 'help cd' for related documentation)

I'm gonna declare this done for now.

@faho faho closed this Mar 26, 2019
@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

4 participants