neogit icon indicating copy to clipboard operation
neogit copied to clipboard

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

Open ascandella opened this issue 3 years ago • 11 comments

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

ascandella avatar Jun 09 '21 22:06 ascandella

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?

TimUntersberger avatar Jun 12 '21 12:06 TimUntersberger

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 :)

ascandella avatar Jun 12 '21 15:06 ascandella

I added two notifications:

  • Committing ... which gets displayed when you start a commit and gets removed once the commit is done.
  • Successfully commited! which gets displayed when the commit is done

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 avatar Jun 12 '21 17:06 TimUntersberger

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.

TimUntersberger avatar Jun 12 '21 17:06 TimUntersberger

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!

ascandella avatar Jun 12 '21 20:06 ascandella

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?

bmulholland avatar Nov 09 '21 11:11 bmulholland

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? 🤔

TimUntersberger avatar Nov 09 '21 12:11 TimUntersberger

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

bmulholland avatar Nov 09 '21 12:11 bmulholland

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.

dkuettel avatar Jun 02 '22 21:06 dkuettel

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

gegoune avatar Jun 02 '22 21:06 gegoune

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:

  • stdout and stderr point to the same table with the same content
  • those tables contain just one entry, one string, with "^M" new lines inserted, instead of a table with a string per line

dkuettel avatar Jun 02 '22 22:06 dkuettel