Separator sometimes shown when there isn't something to separate

[technicalpickles]

🔧 Summary

I have quite a few skip_output options. With the specific combination I use, I've found that there's an extra separator displayed, even though there isn't any prior output to separate

Lefthook version

1.5.0

Steps to reproduce

Here's my skip_output configuration:

skip_output:
  - meta           # Skips lefthook version printing
  # - summary      # Skips summary block (successful and failed steps) printing
  - empty_summary  # Skips summary heading when there are no steps to run
  # - success      # Skips successful steps printing
  # - failure      # Skips failed steps printing
  - execution      # Skips printing any execution logs (but prints if the execution failed)
  - execution_out  # Skips printing execution output (but still prints failed commands output)
  - execution_info # Skips printing `EXECUTE > ...` logging
  - skips          # Skips "skip" printing (i.e. no files matched)
  - meta           # Skips lefthook version printing

Then lefthook run <stage>

Expected results

❯ git push
summary: (done in 9.96 seconds)
✔️  codeowners

Actual results

❯ git push

  ────────────────────────────────────
summary: (done in 9.96 seconds)
✔️  codeowners

Possible Solution

I tracked the code outputting this to:

lefthook/internal/log/log.go

Lines 275 to 289 in c8e4b13

	func Separate(s string) {
	Info(
	lipgloss.JoinVertical(
	lipgloss.Left,
	lipgloss.NewStyle().
	BorderStyle(lipgloss.NormalBorder()).
	BorderBottom(true).
	BorderForeground(colorBorder).
	Width(separatorWidth).
	MarginLeft(separatorMargin).
	Render(""),
	s,
	),
	)
	}

https://github.com/evilmartians/lefthook/blob/c8e4b13eb99bdf02482e6d4bfb4f44a551320162/internal/lefthook/run.go#L184C3-L184C3

I think one way to fix would be to keep track if there was any output displayed before the summary before calling Separate.

Logs / Screenshots

[mrexox]

Hey! Sorry for the long delay. What do you think if we skip summary separator when execution or execution_info and execution_out are set? Sounds like these are the only cases when summary separator is not needed 🤔

[technicalpickles]

That makes sense to me 👍🏻 I only suggested the config option just because I couldn't trace when it was it was actually needed 😅

[mrexox]

I'll release the change next week. Will probably add a few more fixes to the next release 👍

[technicalpickles]

Confirmed this fixed in 1.5.5, thanks!

[avudnez]

Hello, I think this misses some edge cases.

Given the following config

skip_output:
  - meta           # Skips lefthook version printing
    #- summary        # Skips summary block (successful and failed steps) printing
  - empty_summary  # Skips summary heading when there are no steps to run
    #- success        # Skips successful steps printing
    #- failure        # Skips failed steps printing
  - execution      # Skips printing any execution logs (but prints if the execution failed)
  - execution_out  # Skips printing execution output (but still prints failed commands output)
  - execution_info # Skips printing `EXECUTE > ...` logging
  - skips          # Skips "skip" printing (i.e. no files matched)

commit-msg:
  commands:
    this_script_will_error:
      run: python this_script_will_error.py {1}

I have the following output

$ lefthook run commit-msg
Traceback (most recent call last):
  File "this_script_will_error.py", line 104, in <module>
    exit(main())
  File "this_script_will_error.py", line 82, in main
    with open(commit_message_file) as commit_message:
FileNotFoundError: [Errno 2] No such file or directory: '{1}'

exit status 1summary: (done in 0.06 seconds)
🥊  this_script_will_error

The purpose of the hook is not relevant here, but note that it's outputing an error (in this case an exception, but it could be a legit failure message) and two unwanted things occur:

• there is this exit status 1 that I guess is printed by lefthook ?
• it's directly followed by the summary but would benefit from a separator

[mrexox]

@avudnez , do you use the latest version of lefthook?

[avudnez]

Yes, I noticed that when updating from 1.5.2 to 1.5.5