timbrel / GitSavvy

Full git and GitHub integration with Sublime Text
MIT License
1.91k stars 136 forks source link

Properly support git-hooks or (at the very least) document which ones we do not support and why #161

Open joshgoebel opened 9 years ago

joshgoebel commented 9 years ago

Here is a list of the basic (non email patch) git hooks and whether or not we currently support them.

If we kick off rebases using the git rebase command as I suggested then that would automatically add support for pre-rebase at that time. That would leave only prepare-commit-msg as unsupported. Rather than have all the hooks work except for one I'd love to see if we could support this. It seems as simple as asking git to furnish the prepared commit msg (thereby running the prepare-commit-msg automatically in the process) and then plugging that into our commit dashboard work-flow.

It would also remove some of the hand coded variants we have currently contrived to work around the fact that we simply aren't leaning on git for it's own default behavior:

Please consider supporting these hooks. It just feels like when we're rebuilding basic git functionality that we're doing something wrong. I'm even game to do the legwork on prepare-commit-msg as it fits right in to the other commit workflow stuff I've been working on.

joshgoebel commented 9 years ago

Without knowing what you have in store for commit dash it's hard to say with certainty but I'm pretty sure supporting this fully doesn't have to undermine any of that.

  1. Most basically we're talking about fetching the default commit text git thinks should be default (from command line options, hooks, etc)... then that would auto-fill the are that's used for editing your commit message. Simple.
  2. Secondly I could imagine some advanced stuff (and even git itself) using the commented section for special notes or commit aids (showing what you're about to commit, squashed commit history, other custom things)... so I think to fully support that your "dash" controls might need to have a "fall beneath mode" that allows them to come in underneath the full text provided for COMMIT_EDITMSG:
fix: now 3x faster on Intel

# git provided comments fall right here if configured to
# otherwise could be stripped keeping only commit msg
# that git provided (post prepare-commit-msg hooks)

*************************************************
dash board and super awesome stuff here
*************************************************

Hopefully you'll give this some consideration.

divmain commented 9 years ago

Thanks for this.

I think you may be right, at least in part. I made certain choices because it was easier to recreate than to figure out the proper Git plumbing incantation. However, to play nice with the larger Git eco-system (hooks are a great example of this), it would be best to stick close to Git unless there's a compelling reason not to.

So I'll look into using prepare-commit-msg for the dashboard. My guess is that it won't be an active blocker to what I have in mind, but it is difficult to know for sure before I get there.

rwols commented 7 years ago

A pre-commit hook returns exit status 0 but I still get this in a GitSavvy output panel:

`/usr/bin/git commit -q -F -` failed with following output:
On branch add-willsave-waituntil
nothing to commit, working tree clean

Package path: /Users/raoulwols/Library/Application Support/Sublime Text 3/Packages/LSP

## Report for LSP #####################################

No failures

Reporting 2 warnings:
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: Syntaxes/Diagnostics.sublime-syntax
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: Syntaxes/References.sublime-syntax

For more details on the report messages (for example how to resolve them), go to:
https://github.com/packagecontrol/py_package_reviewer/wiki
mauthi commented 6 years ago

Is there an update if you plan to solve the not working "prepare-commit-msg" hook?

taubi19 commented 6 years ago

We would be very interested in this feature, as we are manually writing the branch name on the beginning of each commit.

oerp-odoo commented 6 years ago

Does pre-commit work? I wanted it to work when using SublimeGit, but its not working there, so I was hoping GitSavvy had solved that problem. But it seems this plugin has same issue.

If call git commit from terminal (that opens sublime), pre-commit hook works, but if I do it directly from sublime (using gitsavvy), it ignores the hook. And it is only triggered after I try to do commit. So it works more like post-commit hook, instead of pre-commit.

It seems this issue has to do with using git commit -F mode, which does not fire pre-commit.

P.S. You can find that issue discussed here: https://github.com/SublimeGit/SublimeGit/issues/73

stoivo commented 6 years ago

It doesnt work like in the terminal since in GitSavvy you make the commit message before GitSavvy call git commit ...., we pass the commit message in as STDIN. That was to do we us wanting to controll the content on the commit view. We have settings for showing diffs, stats or none.

Since this is a outbut in real time I would suggust to turn on live output. Add this section to your GitSavvy settings. This setting is recomanded if you have pre push hooks since you want to see the progress of the push/hook.

    "show_stdin_in_output": true,
    "show_panel_for": ["commit"],

If you want the precommit hook to run before you can type the message. Create a custom command, I think this should work for you.

    {
        "caption": "git: commit raw",
        "command": "gs_custom",
        "args": {
            "args": ["commit"],
            "start_msg": "Started commit",
            "complete_msg": "Finished commit",
            "custom_environ": {"EDITOR": "/usr/local/bin/subl -nw"}
        }
    },

Please report back if it works or not. I have zero experience with pre-commit hook so, please tell me how it works if thos custom command doen't work.

oerp-odoo commented 6 years ago

Hmm, I tried creating command by creating file User.sublime-commands in my sublime User directory, but it seems that command is not picked up and I can only see original commit command from GitSavvy (when using command panele -> CTRL+SHIT+p). Maybe I'm missing something here?

Though as workaround, what I do now, I use GitSavve commit command and after that I immediately try to commit it (even when I have no message written yet), then it triggers pre commit and it shows me if pre-commit found any errors in diff I'm about to commit. Whats good there, is then pre-commit is fired sooner than warning about having not entered any message.

So if I see warning about empty message, I know pre-commit passed correctly, and I can proceed with my message (because commit message window from sublime is not closed even if non zero exit code is returned) and If I see pre-commit checks not passing, I know I need to cancel my commit and fix the code.

Well its a bit clunky that I actively need to try commit when I should not do it yet technically (I have not written commit message at that point), but that is what I currently came up with.

P.S. Oh and I'm using "show_panel_for": ["commit"], so it shows output of commit command (like what was commited, what pre-commit outputed etc.), but it shows even if I not use "show_stdin_in_output": true,. stdin at least in those cases (when pre-commit is fired or commit message is empty) does not show anything at all. I only saw it showing original commit message entered when I successfully amend commit (it shows in unformatted string with \n and so on), but that was not really useful for me, so I disabled that output.

randy3k commented 6 years ago

As a fix, we should execute .git/hooks/pre-commit before the creation of the commit window and execute the commit command as git commit --no-verify do suppress the pre-commit hook from running again.

Edit: with --no-verify, the commit_msg hook is also suppressed.

This might be another solution.

randy3k commented 6 years ago

@oerp-odoo would you checkout #982 to see if it works for you?