Passing git hook args to the scripts

[Apemb]

This project is very nice, thank you for your time and effort :-)

I was considering using it in my current project, but one of our usecase is to append commit message with the branch ticket number (example: my branch is feature/23_awesome_feature I want my commit to be named #23 Nice commit message) and one of the way to achieve that is to use the commit-msg git hook.

This hook receives one argument, the temporary commit message file path, and expects the script to modify the file and return 0 (success code) or anything else (error code).

Except if I am mistaken, but here the scripts do not receive any args. Is there any reason for that ? I would gladly do a pull request to add that feature if you want :) .

[qgadrian]

Hey @Apemb!

Yeah definitely this project needs to support the commit-msg hook properly.

Let me know if you want to work on a PR, otherwise I will try to find some time this week to fix the open issues :)

I just update the project so you can use {:file, "path_to_script"} and you will receive the git arguments as usual.

Under priv/test_script you will have a small example that will echo all the args for the configured hook (you can see in the README the config format).

I published a preview version 0.3.2-pre that I will try to validate on my projects, feel free to take a look at give your input.

Thanks for the contribution!

[Apemb]

You Sir are awesome ! I upgraded my version and all seems to work perfectly with {:file, "path_to_script"}. I arrived this morning thinking to work on a nice PR and you already made it possible :-).

If possible I would suggest to make the mix tasks receive the arguments too. Creating a mix task is quite easy, easier than create a bash script for me at least.
This might allow also to not have to explicitly ask for the args like this {:file, "path_to_script"}. If the script is executable and the args are transferred any way path_to_script should be enough no ? (for now it launches the script but the arguments do not seem to be transferred)
Is there a technical hurdle to do it this way ? Something I do not see ?

Anyway thanks again, if I can help you in anyway please tell me

[qgadrian]

Sure, what about adding a {:mix_task, "task_name"} that will invoke the task with the hook arguments (if there are)?

By default, having a list of strings will run the plain commands, so we need to differentiate when we can send the hook arguments and when we cannot.

This will be really easy to implement, I'll have a new version by the end of today so you can give it a try :)

[Apemb]

So juste for me to understand, that would mean three modes :

• arbitrary command without arguments
• arbitrary mix task with arguments
• file path with arguments

Wouldn't it be better to have something like {:cmd_with_args, "mix task_name"} instead of {:mix_task, "task_name"} ? (since the default string command argument is reacting like {:cmd_without_args, "bash command"}
{:cmd_with_args, "bash command"} is the command with argument added to the command after bash command arg1 arg2 ?
Because probably {:mix_task, "task_name"} should react as the default arbitrary command to execute non ? Like {:cmd_with_args, "mix task_name"} ?

That would mean :

• arbitrary command without arguments: "mix task_name"
• arbitrary command with arguments appended after: {:cmd_with_args, "mix task_name"}
• file path with arguments: {:file, "path_to_script"}

What do you think ? It is not a big change, I am not entirely sure it is worth the time. It was more to see if the interface could be standardized between the three modes, all of them being a tuple, with syntactic sugar for the default {:cmd_without_args, "mix task_name"} where one could only write a string.

The possibility to use a mix task is anyways very nice :) With additional modules like git_cli the task can do some nice things while being tested properly and not having to launch chmod +x on the script.

(for example right now my script file contains one line:

elixir ./priv/scripts/prepend-commit-message.exs $@

which could be put directly into a {:cmd_with_args, "elixir ./priv/scripts/prepend-commit-message.exs "} if the arguments are prepended after that command string)

[qgadrian]

I think it's better to standarize the way the actions are configured. This is, having cmd_with_args, cmd_without_args and file (although maybe we should have with and without args as well...).

I liked the cleaning less of just declaring a list of strings, but clearly it's not flexible enough.

What about having the git hook args filtered via opts? It would look something like this:

{:cmd, "elixir ./priv/scripts/prepend-commit-message.exs", include_hook_args?: true}

[Apemb]

{:cmd, "elixir ./priv/scripts/prepend-commit-message.exs"} and {:file, "./priv/scripts/some_script"} seems like good abstractions. It seems clear to me :)
The include_hook_args?: true as an optional argument that is false by default and that one can put as true if needed seems nice and understandable too !

As of using only strings as command, allowing just strings that ends up being {:cmd, "command", include_hook_args?: false} would be fine IMHO, and has the advantage of making those new features not breaking change 👌

[qgadrian]

Hey @Apemb I just updated the project with the {:cmd, _} option as well. It includes the option include_hook_args to forward them to the command, [see README for examples(https://github.com/qgadrian/elixir_git_hooks#type-of-tasks).

There is another pre-release on hex named 0.3.2-pre3, let me know if it's working fine for you :)

[Apemb]

So I tested your new improvements, and it all works great 🎉 ! (tested with {:cmd, "elixir ./priv/scripts/some_script.exs", include_hook_args?: true}, {:cmd, "mix some_task", include_hook_args?: true}, {:file, "./priv/scripts/some_script", include_hook_args?: true} and {:cmd, "mix format"}. All work as expected 👍 )

I have a small suggestion. When the commands run for exemple this one {:cmd, "mix format"}
The verbose output is

↗ Running hooks for :pre_commit
✔ `mix` was successful

Which is not as helpful as having the whole command written (mix format in that case).
I would make the output contain the complete command. What do you think ?

Edit :
In the documentation, I think the fact that "some command" and {:cmd, "some command"} are equivalent and do the same thing is not really clear, and might need a bit more explanations :)

[qgadrian]

Wonderful :)

Thanks for the other feedback btw, I'll fix it by the weekend and release the new version. I'll open new tickets for them and close this one.

Thanks again @Apemb