preexec doesn't work for function declarations

[evverx]

$ source .bash-preexec.sh
$ preexec() { echo "just typed $1"; }
$ date
just typed date
Wed Jun 17 16:20:15 UTC 2015
$ function foo() { echo foo; }
$ foo
just typed foo
foo
$ (date)
Wed Jun 24 02:42:48 UTC 2015

[rcaloras]

Hey thanks for the feedback! This is a known limitation along with subshell parsing e.g. doing something like

(ls)

I have a branch in progress for this fix, but it's not too easy. https://github.com/rcaloras/bash-preexec/tree/SubShellParsing.

It actually takes a decent amount of time for bash to handle this case and I believe it creates some lag. Would love your contribution if you can help figure it out. I'll give it another look tonight as well.

[evverx]

@rcaloras , thanks. I'll read https://github.com/rcaloras/bash-preexec/tree/SubShellParsing this evening:)

[evverx]

Reminder for me:

$ source .bash-preexec.sh
$ preexec() { echo "just typed $1"; }
$ date
just typed date
Wed Jun 17 17:07:28 UTC 2015
$ <SPACE>who
just typed date
vagrant  pts/1        2015-06-17 17:06 (10.0.2.2)

[rcaloras]

That should address the space issue :) Not sure about functions yet though.

[evverx]

@rcaloras , thanks:)
But man bash says:

A value of ‘ignoreboth’ is shorthand for ‘ignorespace’ and ‘ignoredups’

Overwriting HISTCONTROL is privacy issue:)

[rcaloras]

Yeah it should also replace ignoreboth with just ignoredups.

Why is overwriting histcontrol a privacy issue? This plugin overwrites both bash's DEBUG trap and PROMPT_COMMAND IMO that's more invasive :P

[evverx]

that's the joke:)

Reminder for me:
Some useful variables and options:

     set -o history enable command history

     HISTSIZE
              The number of commands to remember in the command  history.
              If  the value is 0, commands are not saved in
              the history list.  Numeric values less than zero result in every
              command  being  saved  on  the history list (there is no limit).
              The shell sets the  default  value  to  500  after  reading  any
              startup files.

     HISTIGNORE
              A colon-separated list of patterns used to decide which  command
              lines  should  be  saved  on  the history list.

[rcaloras]

Added a commit to handle ignoreboth.

[rcaloras]

Could probably add a few more checks to make sure history is properly enabled. Something like

history_check=$(set -o | grep history)
if [[ "$history_check" == "history off" ]]; then
   echo "Enable history to use bash preexec"
   return 1
fi;

[rcaloras]

I think it might be easier to get functions by checking changes in the bash history then investigating things like sub shell parsing.

[evverx]

@rcaloras , thanks.

Could probably add a few more checks to make sure history is properly enabled

Too many checks: history on, HISTSIZE, history-size in ~/.inputrc...

I vote for auto enabling:)

I've collected some useful settings:

# enable access to the command history
set -o history

# save each line of a multi-line command in the same history entry
shopt -s cmdhist

# save the command with embedded newlines instead of semicolons
shopt -s lithist

# save lines which begin with a space character in the history list
# preserve dups!
HISTCONTROL= # already done

# save all lines in the history list
HISTIGNORE=

# every command being saved on the history list (there is no limit)
HISTSIZE=-1
bind 'set history-size -1'

[evverx]

Another history problem is HISTTIMEFORMAT

$ source .bash-preexec.sh
$ preexec() { echo "just typed $1"; }
$ HISTTIMEFORMAT='%F %T '
just typed HISTTIMEFORMAT='%F %T '
$ echo WAT
just typed 2015-06-22 04:56:43 echo WAT

Possible solution:

local this_command="$(HISTTIMEFORMAT= history 1 | sed -e "s/^[ ]*[0-9]*[ ]*//g")";

[rcaloras]

Want to submit a pull request for that one? Would love to get you some contributor credit :) Feel free to open it as a separate issue if you'd like so we can close it specifically.

[evverx]

Fixed:)

[rcaloras]

@evverx I created a viable work around that's based on history changes to detect sub shell and function executed commands.

Give it a look. I'm testing it out now. I'll merge to master and cut a release if I find it's working fine.

[rcaloras]

Also moved your history related suggestions to #10. Thanks!

[evverx]

Wow!
@rcaloras , thanks.
I'll test this change tomorrow.

[rcaloras]

Bump! @evverx I'm going to merge and cut a version if you're cool with this guy. I've been using it locally for a few days now on my installation of bashhub and all seems good.

[evverx]

@rcaloras , sorry.
There is a problem:

git clone https://github.com/rcaloras/bash-preexec
cd bash-preexec/
git checkout --track origin/FunctionAndSubshells
source bash-preexec.sh
preexec() { echo "just typed $1"; }

function wat { echo wat; } # outputs just typed 'function wat { echo wat; }'. It's OK!
function wat { echo wat; } # nothing here! It's not OK

(date) # outputs just typed '(date)'. It's OK.
(date) # nothing here. It's not OK.

date # outputs just typed 'date'
date # outputs just typed 'date'

[rcaloras]

So here's the problem. Function declarations and subshell commands don't invoke the DEBUG trap. So I instead monitor changes to the history file and invoke preexec when a changes occurs. As such we'd also need to remove ignoring duplicates to detect that someone has declared the same function twice or entered the same subshell command twice.

Although I removed the ignoring for white spaces by default, I'm not sure I'm comfortable with removing ignore duplicates :/

[isp5]

I have a question that seems to be related to some of this discussion. While using this script, the history does not store duplicate commands. Is there any particular reason for that? Forgive me if this is self-explanatory, I possibly haven't looked at it enough to figure it out on my own.

[evverx]

Hi, @isp5 .
You have ignoreboth or ignoredups in the HISTCONTROL variable.
Try HISTCONTROL=.

[isp5]

Thanks! Any particular reason the history was set like that?

[rcaloras]

bash-preexec shouldn't change that aspect of your HISTCONTROL. It will alter it to not ignore whitespace. Ignoredups or ignoreboth were probably set by something else, but if I'm mistaken please let me know :)

[rcaloras]

@gnachman Thanks for the input!

I removed those lines during the original rewrite as I was never able to get them to work correctly. I added them back on the SubShellParsing branch and got it sort of working, but ran into issues with the PS1 variable.

[rcaloras]

Yeah looking back through my comments. It failed on things like prompts that include environment information (e.g. git branch, virtual env)

[gnachman]

I'm using a weird series of hacks on top of the old twistedmatrix code but it behaves itself with a PS1 like this (i.e., preexec and precmd run at the right time):

export PS1='$(git branch) > '

Is that what you meant about prompts that include environment information?

[rcaloras]

@gnachman Yeah exactly. I'm looking through your code now. I'll definitely try to recreate it. Looks like you're doing some interesting stuff with PS1 to work around it e.g. https://github.com/gnachman/iterm2-website/blob/master/source/misc/bash_startup.in#L62-L83.

I actually just had some users of Bashhub try to use Iterm2 + Shell Integration and it failed because we're both trying to install our own versions of preexec :(

I'd love to fix up this project to make it compatible with the awesome iTerm2 :) I wrote bash-preexec to include function arrays and to avoid multiple inclusion which allows multiple shell integrations to add preexec functions triggered through one debug trap hook. Any chance you'd be willing to switch to this library if I make it compatible?

[gnachman]

@rcaloras I would love to not own the preexec code. It hurts my head. The main difficulty is ensuring it works on the ancient version of bash that ships with Mac OS, and the weird hack for CentOS 7.2.

[rcaloras]

@gnachman Awesome, would love to take it off your hands. I'm traveling over the next few weeks, but will take a look at aligning mine with some of your changes when I get back. Thanks! 👍

[rcaloras]

@evverx Addressed the issue for subshells by looking at some of the code from @gnachman. [[ ! -t 1 && -z ]] was the main thing I was was missing 👍

Not positive there's a solution for functions :/ I'm going to release this and another change. Give it a try if you get a chance. If everything looks good I'll rename this issue to just address function declarations.

[evverx]

@rcaloras , works fine! Thanks!

$ git clone https://github.com/rcaloras/bash-preexec
$ source bash-preexec/bash-preexec.sh
$ preexec() { echo "just typed $1"; }
$ (date)
just typed (date)
Sun 24 Apr 08:56:31 UTC 2016

But I don't really understand how:)