8000 Skip pre/post exec events for empty commands (#4829) by awalgarg · Pull Request #7085 · fish-shell/fish-shell · GitHub
[go: up one dir, main page]

Skip to content

Skip pre/post exec events for empty commands (#4829)#7085

Closed
awalgarg wants to merge 1 commit intofish-shell:masterfrom
awalgarg:master
Closed

Skip pre/post exec events for empty commands (#4829)#7085
awalgarg wants to merge 1 commit intofish-shell:masterfrom
awalgarg:master

Conversation

@awalgarg
Copy link
Contributor
@awalgarg awalgarg commented Jun 6, 2020

Description

In the same vein as #4926, firing fish_preexec and fish_postexec for empty commands is inconsistent with the way $status, history and $CMD_DURATION treat empty commands. fish_postexec, for example, receives an incorrect value for $CMD_DURATION when an empty command is processed.

As suggested in #4829, this patch skips firing of these events for empty commands altogether.

Fixes issue #4829

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@faho
Copy link
Member
faho commented Jun 6, 2020

So what are we doing for empty commands? Should we possibly just not be doing anything instead?

@faho faho added this to the fish 3.2.0 milestone Jun 6, 2020
@awalgarg
Copy link
Contributor Author
awalgarg commented Jun 6, 2020

I do think simply ignoring empty commands altogether should be fine, but felt it might be too big a change. Empty commands are often used to "separate" a bunch of commands from the rest for readability, or for some reason to refresh a prompt with a time indicator etc.

This patch simply follows the precedent set from CMD_DURATION to fix the issue with postexec hooks receiving an incorrect duration value. If we are okay with ignoring empty commands altogether, I can just do that then.

@ridiculousfish
Copy link
Member

I agree with faho, let's merge this, and then try skipping even more work for empty commands.

Merged as cb5eb72
Extended as 61e9484

Thank you!

@awalgarg
Copy link
Contributor Author
awalgarg commented Jun 7, 2020

Thanks, @ridiculousfish!

awalgarg added a commit to awalgarg/dotfiles that referenced this pull request Jul 29, 2020
The weird arg length check is due to bug in fish. Sent a patch to fix
it, will be released in fish 3.2.0.

Ref: fish-shell/fish-shell#7085
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0