Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add config options for length of commit hash displayed in commits view #3505

Merged
merged 2 commits into from Apr 28, 2024
Merged

Add config options for length of commit hash displayed in commits view #3505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4354cc
Add config option for length of commit hash displayed in commits view
oliviaBahr Apr 19, 2024
b63321a
Refactor `pkg/gui/presentation/commits.go` slightly to be consistent
stefanhaller Apr 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/Config.md
View file
Open in desktop
Expand Up @@ -80,6 +80,7 @@ gui:
showIcons: false # deprecated: use nerdFontsVersion instead
nerdFontsVersion: "" # nerd fonts version to use ("2" or "3"); empty means don't show nerd font icons
showFileIcons: true # for hiding file icons in the file views
commitHashLength: 8 # length of commit hash in commits view. 0 shows '*' if NF icons aren't enabled
commandLogSize: 8
splitDiff: 'auto' # one of 'auto' | 'always'
skipRewordInEditorWarning: false # for skipping the confirmation before launching the reword editor
Expand Down
3 changes: 3 additions & 0 deletions pkg/config/user_config.go
View file
Open in desktop
Expand Up @@ -123,6 +123,8 @@ type GuiConfig struct {
NerdFontsVersion string `yaml:"nerdFontsVersion" jsonschema:"enum=2,enum=3,enum="`
// If true (default), file icons are shown in the file views. Only relevant if NerdFontsVersion is not empty.
ShowFileIcons bool `yaml:"showFileIcons"`
// Length of commit hash in commits view. 0 shows '*' if NF icons aren't on.
CommitHashLength int `yaml:"commitHashLength" jsonschema:"minimum=0"`
// If true, show commit hashes alongside branch names in the branches view.
ShowBranchCommitHash bool `yaml:"showBranchCommitHash"`
// Height of the command log view
Expand Down Expand Up @@ -675,6 +677,7 @@ func GetDefaultConfig() *UserConfig {
ShowIcons: false,
NerdFontsVersion: "",
ShowFileIcons: true,
CommitHashLength: 8,
ShowBranchCommitHash: false,
CommandLogSize: 8,
SplitDiff: "auto",
Expand Down
42 changes: 29 additions & 13 deletions pkg/gui/presentation/commits.go
View file
Open in desktop
Expand Up @@ -312,9 +312,33 @@ func displayCommit(
bisectInfo *git_commands.BisectInfo,
isYouAreHereCommit bool,
) []string {
hashColor := getHashColor(commit, diffName, cherryPickedCommitHashSet, bisectStatus, bisectInfo)
bisectString := getBisectStatusText(bisectStatus, bisectInfo)

hashString := ""
hashColor := getHashColor(commit, diffName, cherryPickedCommitHashSet, bisectStatus, bisectInfo)
hashLength := common.UserConfig.Gui.CommitHashLength
if hashLength >= len(commit.Hash) {
hashString = hashColor.Sprint(commit.Hash)
} else if hashLength > 0 {
hashString = hashColor.Sprint(commit.Hash[:hashLength])
} else if !icons.IsIconEnabled() { // hashLength <= 0
hashString = hashColor.Sprint("*")
}

divergenceString := ""
if commit.Divergence != models.DivergenceNone {
divergenceString = hashColor.Sprint(lo.Ternary(commit.Divergence == models.DivergenceLeft, "↑", "↓"))
} else if icons.IsIconEnabled() {
divergenceString = hashColor.Sprint(icons.IconForCommit(commit))
}

descriptionString := ""
if fullDescription {
descriptionString = style.FgBlue.Sprint(
utils.UnixToDateSmart(now, commit.UnixTimestamp, timeFormat, shortTimeFormat),
)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I'd set hashString to the uncolored string, and use hashColor.Sprint in the append call below as before. Seems more logical to me, somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did hashColor.Sprint there to be consistent with most of the rest of that function where Sprint is used to build the strings.

lazygit/pkg/gui/presentation/commits.go

Lines 326 to 399 in 961c3bc

actionString := ""
if commit.Action != models.ActionNone {
todoString := lo.Ternary(commit.Action == models.ActionConflict, "conflict", commit.Action.String())
actionString = actionColorMap(commit.Action).Sprint(todoString) + " "
}
tagString := ""
if fullDescription {
if commit.ExtraInfo != "" {
tagString = style.FgMagenta.SetBold().Sprint(commit.ExtraInfo) + " "
}
} else {
if len(commit.Tags) > 0 {
tagString = theme.DiffTerminalColor.SetBold().Sprint(strings.Join(commit.Tags, " ")) + " "
}
if branchHeadsToVisualize.Includes(commit.Hash) &&
// Don't show branch head on commits that are already merged to a main branch
commit.Status != models.StatusMerged &&
// Don't show branch head on a "pick" todo if the rebase.updateRefs config is on
!(commit.IsTODO() && hasRebaseUpdateRefsConfig) {
tagString = style.FgCyan.SetBold().Sprint(
lo.Ternary(icons.IsIconEnabled(), icons.BRANCH_ICON, "*") + " " + tagString)
}
}
name := commit.Name
if commit.Action == todo.UpdateRef {
name = strings.TrimPrefix(name, "refs/heads/")
}
if parseEmoji {
name = emoji.Sprint(name)
}
mark := ""
if isYouAreHereCommit {
color := lo.Ternary(commit.Action == models.ActionConflict, style.FgRed, style.FgYellow)
youAreHere := color.Sprintf("<-- %s ---", common.Tr.YouAreHere)
mark = fmt.Sprintf("%s ", youAreHere)
} else if isMarkedBaseCommit {
rebaseFromHere := style.FgYellow.Sprint(common.Tr.MarkedCommitMarker)
mark = fmt.Sprintf("%s ", rebaseFromHere)
} else if !willBeRebased {
willBeRebased := style.FgYellow.Sprint("✓")
mark = fmt.Sprintf("%s ", willBeRebased)
}
authorFunc := authors.ShortAuthor
if fullDescription {
authorFunc = authors.LongAuthor
}
cols := make([]string, 0, 7)
if commit.Divergence != models.DivergenceNone {
cols = append(cols, hashColor.Sprint(lo.Ternary(commit.Divergence == models.DivergenceLeft, "↑", "↓")))
} else if icons.IsIconEnabled() {
cols = append(cols, hashColor.Sprint(icons.IconForCommit(commit)))
}
cols = append(cols, hashString)
cols = append(cols, bisectString)
if fullDescription {
cols = append(cols, style.FgBlue.Sprint(
utils.UnixToDateSmart(now, commit.UnixTimestamp, timeFormat, shortTimeFormat),
))
}
cols = append(
cols,
actionString,
authorFunc(commit.AuthorName),
graphLine+mark+tagString+theme.DefaultTextColor.Sprint(name),
)
return cols
}

I like this format and would actually go further and make the few strings that were built inside the cols construction match the rest. 389a8c9.

The cols construction ends up looking like this

lazygit/pkg/gui/presentation/commits.go

Lines 392 to 405 in 389a8c9

cols := make([]string, 0, 7)
cols = append(
cols,
divergenceString,
hashString,
bisectString,
descriptionString,
actionString,
authorFunc(commit.AuthorName),
graphLine+mark+tagString+theme.DefaultTextColor.Sprint(name),
)
return cols
}

This commit doesn't change the functionality at all but I think the consistency makes it more readable. I haven't written in go before so maybe there was a reason it was like that. No need to include this commit in the pull, just a suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I don't care too much about which way the code is written here, so I'm ok with your refactoring, but I don't really think it's necessary. One thing to be aware of is that now you are unconditionally adding divergence and description, whereas previously they were only added when needed. So now you are relying on the remove-empty-columns mechanism to remove them again; this works, but is unnecessary work.

My point about the hashString was that I don't like how the logic of assembling the string is mixed with coloring it. Not a huge deal either, I think we're talking about all this for longer than justified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly you'd prefer something like

	cols = append(
		cols,
		hashColor.Sprint(divergenceString),
		hashColor.Sprint(hashString),
		// etc.
	)

I think I agree I prefer that logic as well, but like I said I did it the way I did because all of the rest of the strings in that function are colored and formatted where they are assembled. It felt weird to have just those one or two strings be colored in the append call.

actionString := ""
if commit.Action != models.ActionNone {
todoString := lo.Ternary(commit.Action == models.ActionConflict, "conflict", commit.Action.String())
Expand Down Expand Up @@ -368,20 +392,12 @@ func displayCommit(
}

cols := make([]string, 0, 7)
if commit.Divergence != models.DivergenceNone {
cols = append(cols, hashColor.Sprint(lo.Ternary(commit.Divergence == models.DivergenceLeft, "↑", "↓")))
} else if icons.IsIconEnabled() {
cols = append(cols, hashColor.Sprint(icons.IconForCommit(commit)))
}
cols = append(cols, hashColor.Sprint(commit.ShortHash()))
cols = append(cols, bisectString)
if fullDescription {
cols = append(cols, style.FgBlue.Sprint(
utils.UnixToDateSmart(now, commit.UnixTimestamp, timeFormat, shortTimeFormat),
))
}
cols = append(
cols,
divergenceString,
hashString,
bisectString,
descriptionString,
actionString,
authorFunc(commit.AuthorName),
graphLine+mark+tagString+theme.DefaultTextColor.Sprint(name),
Expand Down
6 changes: 6 additions & 0 deletions schema/config.json
View file
Open in desktop
Expand Up @@ -309,6 +309,12 @@
"description": "If true (default), file icons are shown in the file views. Only relevant if NerdFontsVersion is not empty.",
"default": true
},
"commitHashLength": {
"type": "integer",
"minimum": 0,
"description": "Length of commit hash in commits view. 0 shows '*' if NF icons aren't on.",
"default": 8
},
"showBranchCommitHash": {
"type": "boolean",
"description": "If true, show commit hashes alongside branch names in the branches view."
Expand Down