NeogitOrg / neogit

An interactive and powerful Git interface for Neovim, inspired by Magit
MIT License
4.07k stars 240 forks source link

Discussion: viewing output of git commit (e.g. for commit hooks) #139

Closed ascandella closed 1 week ago

ascandella commented 3 years ago

Hey there,

I have a repo that I work on that has some git hooks (husky, with lint-staged, eslint, prettier/pretty-quick) that take a few seconds to run (and occasionally fail). I wanted to see how hard it would be to add support for viewing commit hook output, so I spent a few hours and put together a proof-of-concept: https://github.com/ascandella/neogit/pull/1

I realize that you can currently view failed command output with $, but the idea here is to provide some feedback on commit progress (even in the success case).

I don't think this should in any way be merged as-is, but I was curious what the sentiment around here is for something like this. Does this complicate the code for the benefit of very few people? Is it something that might be useful for other use cases?

Here's the demo gif, more info in the above PR:

Animation2

TimUntersberger commented 3 years ago

Would it be okay for you if you would have to open the cli history $ to see the command output?

We could add new events so you could automatically open the command output yourself.

If this is acceptable to you I'd like to wait for #131 to be merged first, since this one includes a massive ui refactor which we would benefit a lot from for this usecase.

Regarding more feedback:

We could add a success popup similar to pushing/pulling to commits.

Is there any other part where you feel like some feedback is lacking?

ascandella commented 3 years ago

I don't think I did a very good job explaining that the desire for this feature (for me) is to see progress of the commit.

The gif above is from a test repo I created so that I could post publicly. On my work repo, the lint step takes ~8 seconds if there are any typescript/tsx files staged. Because it takes so long (and does sometimes fail), I like to make sure my commit has succeeded before I move onto the next thing (e.g. close Neogit). In the meantime, after saving the commit buffer, it's helpful to me to have some feedback about what's going on (seeing the status of the hooks).

So I'm not sure if this is a problem that I've created in my head that only exists for a very small number of users (possibly only me), or something that other people may find useful.

I don't actually care so much about the textual output of the commit hooks (because if they failed, it's pretty easy from the $ buffer to find it, or just using neovim/lsp diagnostics). It's that I want progress, both in the success and failure case (but usually the success case) that something is happening with my commits.

I think in lieu of actually streaming the output, another way to approach this would be to have a text "spinner" that you could enable like "Committing..." that shows up where the popup does.

But I have no idea how other people feel about this kind of functionality, and definitely don't want to bloat your wonderful plugin if it's just my neurosis :)

TimUntersberger commented 3 years ago

I added two notifications:

One problem is that we delete all notifications when closing neogit, because neovim doesn't allow transfering floating windows across tabs AFAIK, so we have to remove them before you close neogit.

TimUntersberger commented 3 years ago

I think in lieu of actually streaming the output, another way to approach this would be to have a text "spinner" that you could enable like "Committing..." that shows up where the popup does.

Regarding this, I'd suggest to wait until #131 is merged and I have a prototype for the thing I mentioned above and then we can discuss this more.

ascandella commented 3 years ago

Cool, I just tested out your changes to master and this looks great for my use case (non-instantaneous commits).

If people find it annoying we can add a config option to disable it, which I'm happy to do (since, again, I'm not sure if these messages are something that only I want)

I'll keep an eye on 131 :)

Thanks!

bmulholland commented 3 years ago

The alerts (committing and success/failure) are working, but have some issues on failure ("Success" shows even on fail, overlapping fail state):

Screen Shot 2021-11-09 at 12 06 26

Further, I cannot see the reason for failure: pressing $ (as the notification suggests) shows only commands like git status and git ls-files without any output from the commit. Did the way to see commit output change?

TimUntersberger commented 3 years ago

Further, I cannot see the reason for failure: pressing $ (as the notification suggests) shows only commands like git status and git ls-files without any output from the commit. Did the way to see commit output change?

This makes me think that maybe it didn't actually fail, just that we showed the error alert? 🤔

bmulholland commented 3 years ago

No, it failed. And when I try an external (CLI) git commit, I see the hook run and the error message there.

dkuettel commented 2 years ago

Bringing this up again because it's quite misleading that the last message you see is "success" even though it failed. And there is no streamlined way to get to the commit hook output. In my experience commit hooks for to check formatting or similar are quite common and it would be useful to get a handle on the log message.

gegoune commented 2 years ago

Could also apply to push (and pull?) where remote can send messages. For example GitHub giving link to open PR.

dkuettel commented 2 years ago

To elaborate more on "having a handle on it": a) be able to register a callback for when a commit fails (non-zero exit code) b) get access to the command, exit-code, and output

If there is a premade thing that shows the output nicely that's of course welcomed. But people have different tools and they might want to parse the output themselves because they know what to do with it.

See below how I do it now. Just a cheap example. It works and I plan to just parse the output so that I get a list of files that are not well formatted.

local function suck_out_lines(lines)
    local more = {}
    for i = 1, #lines do
        local splitted = vim.split(lines[i], "\r")
        for j = 1, #splitted do
            table.insert(more, splitted[j])
        end
    end
    return more
end

local function view(command, exit_code, stdout, stderr)
    vim.cmd("tabnew")
    vim.api.nvim_buf_set_lines(0, 0, -1, false, { "******************" })
    vim.api.nvim_buf_set_lines(0, -1, -1, false, { command })
    vim.api.nvim_buf_set_lines(0, -1, -1, false, { "******************" })
    vim.api.nvim_buf_set_lines(0, -1, -1, false, { tostring(exit_code) })
    vim.api.nvim_buf_set_lines(0, -1, -1, false, { "******************" })
    vim.api.nvim_buf_set_lines(0, -1, -1, false, suck_out_lines(stdout))
    vim.api.nvim_buf_set_lines(0, -1, -1, false, { "******************" })
    vim.api.nvim_buf_set_lines(0, -1, -1, false, suck_out_lines(stderr))
    vim.api.nvim_buf_set_lines(0, -1, -1, false, { "******************" })
end

local function show_last_commit_log()
    local history = require("neogit.lib.git.cli").history
    for i = #history, 1, -1 do
        local item = history[i]
        if string.sub(item.cmd, 1, 10) == "git commit" then
            view(item.raw_cmd, item.code, item.stdout, item.stderr)
        end
    end
end

Two things I noticed:

CKolkey commented 1 week ago

We now support showing the output of git-hooks (auto-detected), in the console. They can be interrupted via c-c if desired.