Conversation
|
Played around with this and it is looking good. Should be swap out the error messaging when a user inputs an issue number or non-existent PR? This is the current output: |
|
ha yeah should def fix that, oops |
|
Looking good now, thanks! |
probablycorey
left a comment
probablycorey
left a comment
There was a problem hiding this comment.
Looks good and works well! I left some minor naming suggestions inline.
command/pr_diff.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| if !validColor(color) { |
There was a problem hiding this comment.
I like how you pulled this logic into a new function. Maybe change the name to something like validColorFlag, otherwise it sounds like it is validating if the string is an actual color.
| return fmt.Errorf("could not determine base repo: %w", err) | ||
| } | ||
|
|
||
| // begin pr resolution boilerplate |
command/pr_diff.go
Outdated
| case bold(diffLine): | ||
| output = utils.Bold(diffLine) | ||
| case green(diffLine): | ||
| output = utils.Green(diffLine) | ||
| case red(diffLine): |
There was a problem hiding this comment.
Maybe naming these function with the line content would better. So instead of green it would be isAdditionLine.
For the bold it could be something like isHeaderLine
4 participants
Summary
closes #917
This is a first pass on a
gh pr diffcommand. It can be invoked on the current branch's PR, a URL,or a pr number.
It does not call out to a pager like
git diffdoes.You can control its use of color with
--color={always|never|auto}. It won't colorize innon-interactive settings.