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

'commandline -f' interprets functions in reverse #1567

Closed
favdish opened this Issue Jul 19, 2014 · 4 comments

Comments

Projects
None yet
4 participants
@favdish

favdish commented Jul 19, 2014

This doesn't work correctly, since it only performs the execute call:
bind \em 'commandline -f accept-autosuggestion; commandline -f execute'
Reversing the order of the calls does work:
bind \em 'commandline -f execute; commandline -f accept-autosuggestion'
bind \em 'commandline -f execute accept-autosuggestion'
Strange... is that a bug or a feature?

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jul 19, 2014

Member

commandline -f invokes input_unreadch, which pushes commands onto a stack. Stacks are LIFO, so that explains this observation.

I think this dates back to fish 1.x. I would call it a bug - commands ought to be a queue, not a stack.

Member

ridiculousfish commented Jul 19, 2014

commandline -f invokes input_unreadch, which pushes commands onto a stack. Stacks are LIFO, so that explains this observation.

I think this dates back to fish 1.x. I would call it a bug - commands ought to be a queue, not a stack.

@favdish

This comment has been minimized.

Show comment
Hide comment
@favdish

favdish Jul 20, 2014

Well, once it's clear that the functions are called in a stack-like fashion it's NBD. A hint in the docs would've been great... would have saved me some swearing:-)

favdish commented Jul 20, 2014

Well, once it's clear that the functions are called in a stack-like fashion it's NBD. A hint in the docs would've been great... would have saved me some swearing:-)

@zanchey zanchey added the bug label Jul 22, 2014

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jul 29, 2014

Member

I want to try to fix this before the compatibility concerns mean we can't.

Member

ridiculousfish commented Jul 29, 2014

I want to try to fix this before the compatibility concerns mean we can't.

@terlar

This comment has been minimized.

Show comment
Hide comment
@terlar

terlar Aug 21, 2014

Contributor

It also seems like you cannot execute any other operation after commandline -f execute. If you would run the following:

function __execute_and_keep_line
  set -l pos (commandline -C)
  set -l cmd (commandline -b)

  commandline -f execute
  commandline -r "$cmd"
  commandline -C $pos
end
bind \e\n __execute_and_keep_line

No matter which order you run it, it always stops at execute and don't run other stuff after.

Contributor

terlar commented Aug 21, 2014

It also seems like you cannot execute any other operation after commandline -f execute. If you would run the following:

function __execute_and_keep_line
  set -l pos (commandline -C)
  set -l cmd (commandline -b)

  commandline -f execute
  commandline -r "$cmd"
  commandline -C $pos
end
bind \e\n __execute_and_keep_line

No matter which order you run it, it always stops at execute and don't run other stuff after.

@zanchey zanchey modified the milestones: next-minor, next-major Mar 12, 2015

snnw added a commit to snnw/fish-shell that referenced this issue Apr 6, 2015

Change lookahead_list into a queue
Using builtin `commandline -f`, one would expect to have commands executed in
the order that they were given.  This motivates the change to a queue.

Unfortunately, fish internals still need lookahead_list to act as a stack.  Add
and rename functions to support both cases and have lookahead_list as
a std::deque internally.

This code is delicate, and we should probably dog-food this in nightly for
a while before the next-minor release.

Fixes #1567

snnw added a commit to snnw/fish-shell that referenced this issue Apr 6, 2015

Change lookahead_list into a queue
Using builtin `commandline -f`, one would expect to have commands executed in
the order that they were given.  This motivates the change to a queue.

Unfortunately, fish internals still need lookahead_list to act as a stack.  Add
and rename functions to support both cases and have lookahead_list as
a std::deque internally.

This code is delicate, and we should probably dog-food this in nightly for
a while before the next-minor release.

Fixes #1567

snnw added a commit to snnw/fish-shell that referenced this issue Apr 17, 2015

Change lookahead_list into a queue
Using builtin `commandline -f`, one would expect to have commands executed in
the order that they were given.  This motivates the change to a queue.

Unfortunately, fish internals still need lookahead_list to act as a stack.  Add
and rename functions to support both cases and have lookahead_list as
a std::deque internally.

This code is delicate, and we should probably dog-food this in nightly for
a while before the next-minor release.

Fixes #1567
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment