RichiH/vcsh

Hooks for {pre,post}-{pull,push} don't make sense currently.

Closed this issue · 2 comments

lyda commented

To take pull as an example, this is what happens with a vcsh -v pull with a few extra verbose calls thrown in:

verbose mode on
vcsh 1.20151229
vcsh: verbose: pull begin
vcsh: verbose: Running hooks in '/Users/kevin/.config/vcsh/hooks-enabled' for ''
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/pre-command*'
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/.pre-command*'
vcsh: verbose: Running hooks in '/Users/kevin/.config/vcsh/hooks-enabled' for ''
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/pre-pull*'
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/.pre-pull*'
home: Already up-to-date.

past: Already up-to-date.

vcsh: verbose: Running hooks in '/Users/kevin/.config/vcsh/hooks-enabled' for 'past'
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/post-pull*'
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/past.post-pull*'
vcsh: verbose: Running hooks in '/Users/kevin/.config/vcsh/hooks-enabled' for 'past'
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/post-command*'
vcsh: verbose: looking for '/Users/kevin/.config/vcsh/hooks-enabled/past.post-command*'
vcsh: verbose: pull end, exiting

Notice that it looks for a .config/vcsh/hooks-enabled/.pre-pull and a .config/vcsh/hooks-enabled/past.post-pull?

This is because...

pull() {
        hook pre-pull
        for VCSH_REPO_NAME in $(list); do
                printf '%s: ' "$VCSH_REPO_NAME"
                GIT_DIR=$VCSH_REPO_D/$VCSH_REPO_NAME.git; export GIT_DIR
                use
                git pull
                VCSH_COMMAND_RETURN_CODE=$?
                echo
        done
        hook post-pull
}

...the $VCSH_REPO_NAME var remains set after the for loop.

For me a per-repo pre-pull hook would be useful. I'm going to submit a pull-request with one option, but I can see arguments for others. My way will involve adding two new hook functions - a global_hook and a repo_hook. I'll fix the push and pull commands but I think this would be useful else where.

ao2 commented

I see your point but maybe the issue can be split in two: a bug fix, and a new feature.

In one commit the code would uset VCSH_REPO_NAME after the for loops, to fix the asymmetric behavior you highlighted.

After that, per-repo and global hooks differentiation as per your idea from #239 can be tackled based on the fixed code.

This approach could make it clearer and more convincing why such differentiation is desired.

Just an idea.

lyda commented

I maintain my own fork that works as expected - based on #239. So I have a solution. Whatever works for you is fine.