diff --git a/pkg/gui/controllers/helpers/diff_line_raw_fallback.go b/pkg/gui/controllers/helpers/diff_line_raw_fallback.go index 6f967cd64..ee5d41ab2 100644 --- a/pkg/gui/controllers/helpers/diff_line_raw_fallback.go +++ b/pkg/gui/controllers/helpers/diff_line_raw_fallback.go @@ -33,6 +33,21 @@ func (self *DiffLineHelper) MainViewDiffMode() git_commands.DiffMode { return git_commands.DiffModeRendered } +// DiffRowsCanBePlaced reports whether the rows of the diff the main view is about to be +// given can be placed in the file they show. git's own diff describes itself, whether it +// is what the user configured or what MainViewDiffMode is about to substitute for a +// rendering that can't be acted on; any other rendering says where its rows belong only +// if it states records for them. +// +// It is what anything that means to go from a row back to the file it shows has to ask +// first: with neither records nor a diff that describes itself, there is nothing to go +// on, and offering the user the way there would be offering nothing. +func (self *DiffLineHelper) DiffRowsCanBePlaced() bool { + return !self.diffNeedsMetadata() || + self.MainViewDiffMode() == git_commands.DiffModeRaw || + self.diffRendererEmitsMetadata() +} + // RenderFocusedMainViewAgain has the panel beneath the focused main view render its // diff again — which, the main view now holding focus, is git's own diff rather than // the renderer's — and calls place once that is on screen. diff --git a/pkg/gui/controllers/helpers/diff_stat_links.go b/pkg/gui/controllers/helpers/diff_stat_links.go new file mode 100644 index 000000000..a1ffacd87 --- /dev/null +++ b/pkg/gui/controllers/helpers/diff_stat_links.go @@ -0,0 +1,339 @@ +package helpers + +import ( + "bytes" + "io" + "regexp" + "strings" + "sync/atomic" + + "github.com/jesseduffield/lazygit/pkg/gui/style" + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" +) + +// A diff opens with a diffstat naming every file in it, above the diff of each of +// them. Here each of those names is made a link to where that file's diff begins, so +// that a file of a long diff can be gone to by clicking the line that names it. +// +// The names are recognized in the output as it is written to the pane, where they cost +// next to nothing to find. The diffstat is git's own text whichever renderer the diff +// goes through — delta, diff-so-fancy and difftastic all pass it on untouched — and it +// comes first, so the scan for it ends with it. + +// DiffStatLinkScheme names a link to a file of the diff the pane is showing, as +// lazygit-edit names one that opens a file in the editor. The link is never handed to +// the terminal — gocui takes the escape sequence out of the content and gives the URL +// back when the cell it covers is clicked — so the path in it needs no escaping. +const DiffStatLinkScheme = "lazygit-diff-file://" + +// diffStatEntryPattern matches a line of a diffstat and captures the path it states. +// Such a line holds the name of the file, padded out to the width of the longest, then +// the number of lines it changes (or "Bin" for a binary file) and the graph of them. +// +// The name is captured greedily, so that the separator found is the last one on the +// line rather than one in a file name that contains " | " itself. +var diffStatEntryPattern = regexp.MustCompile(`^ (.*[^ ]) +\| +(?:Bin|\d+)`) + +// DiffStatLinkWriter hands a pane's content on to it, turning the file names in the +// diffstat the content opens with into links (see DiffStatLinkScheme). +type DiffStatLinkWriter struct { + writer io.Writer + + // Whether the diffstat is still to come, is being written now, or is behind us. It + // is behind us once a line comes that is no entry of it, or that the diff proper + // begins with, and nothing past that is looked at. A name down there names the file + // the reader is already in. + // + // It is atomic because the render is begun on the UI thread while the content of + // it arrives on the goroutine reading the command's output. + state atomic.Int32 +} + +type diffStatState int32 + +const ( + diffStatToCome diffStatState = iota + inDiffStat + diffStatDone +) + +func NewDiffStatLinkWriter(writer io.Writer) *DiffStatLinkWriter { + return &DiffStatLinkWriter{writer: writer} +} + +// BeginRender starts a fresh render, whose own diffstat is the one to look for. It is +// called as the render is asked for, before any of it is written. +// +// linkFiles says whether this render is one whose file names lead anywhere: the pane's +// own diff, whose rows can be placed in the files they show. Either is passed through +// untouched without it. Content that is no diff of the panel's — a commit log, a +// message — has no diffstat in it, and a line of one that happens to read like an entry +// of a diffstat names no file to go to. A rendering whose rows nothing can place does +// have the files in it, but nothing to find the one a name stands for with. +func (self *DiffStatLinkWriter) BeginRender(linkFiles bool) { + self.setState(lo.Ternary(linkFiles, diffStatToCome, diffStatDone)) +} + +func (self *DiffStatLinkWriter) getState() diffStatState { + return diffStatState(self.state.Load()) +} + +func (self *DiffStatLinkWriter) setState(state diffStatState) { + self.state.Store(int32(state)) +} + +func (self *DiffStatLinkWriter) Write(p []byte) (int, error) { + linked := self.withFileNameLinked(p) + + written, err := self.writer.Write(linked) + if err != nil { + return 0, err + } + if written < len(linked) { + return 0, io.ErrShortWrite + } + // The caller is owed an answer about what it gave us, not about what we passed on. + return len(p), nil +} + +// withFileNameLinked returns the given line of the render with the name in it linked, +// where the line is an entry of the diffstat. +func (self *DiffStatLinkWriter) withFileNameLinked(line []byte) []byte { + state := self.getState() + if state == diffStatDone { + return line + } + + if beginsTheDiffItself(line) { + // A diffstat that hasn't come by now isn't coming: the pane is showing a diff + // that was asked for without one. + self.setState(diffStatDone) + return line + } + + match := diffStatEntry(line) + if match == nil { + if state == inDiffStat { + self.setState(diffStatDone) + } + return line + } + self.setState(inDiffStat) + + start, end := match[2], match[3] + // The link states the name as it reads on screen, so that a renderer that colors + // the diffstat doesn't put escape sequences into it. + name := utils.Decolorise(string(line[start:end])) + linked := make([]byte, 0, len(line)+len(name)+32) + linked = append(linked, line[:start]...) + linked = append(linked, style.PrintHyperlink(string(line[start:end]), DiffStatLinkScheme+name)...) + return append(linked, line[end:]...) +} + +// diffLineRecordOpener opens an OSC 1717 record, ahead of the version whose fields the +// record states (see parseDiffLineMetadata). +const diffLineRecordOpener = "\x1b]1717;" + +// beginsTheDiffItself reports whether the line is one of the diff proper rather than +// one of the diffstat above it: git's own header for a file, or a line a renderer +// states a record about (see statesADiffLine). +func beginsTheDiffItself(line []byte) bool { + return bytes.HasPrefix(line, []byte("diff --")) || statesADiffLine(line) +} + +// diffLineRecordKinds are the kinds of record a renderer states about a line of the +// diff itself. A line stating one of them is below the whole diffstat, which is what +// makes them the end of the search for it. +// +// They are listed here rather than read off the parser's table (see +// diffLineTypeFromMetadata), which answers a different question: whether a record can be +// read at all. A kind the protocol gains for something above the diff — one renderer +// stated the commit line — would belong in that table and not in this list, and taking +// the table for this would end the search where the diffstat hasn't even begun. Every +// kind of the protocol is held against this list by a test. +var diffLineRecordKinds = []string{"c", "a", "d", "f", "h"} + +// statesADiffLine reports whether the line carries a record in which a diff renderer +// states which line of which file it is rendering. Those records are about the lines of +// the diff, and the diffstat comes before all of them. +// +// The kind the record states has to be one of diffLineRecordKinds. A record of any +// other kind says nothing about where the diffstat ends, so the search goes on past it. +// +// The version the record opens with is passed over rather than read. This asks which +// lines a renderer states records for, and the answer holds whichever version of the +// protocol it speaks. A record with nothing after the version is the handshake a +// renderer announces itself with, which is about no line, so the search goes on past it. +func statesADiffLine(line []byte) bool { + for rest := line; ; { + at := bytes.Index(rest, []byte(diffLineRecordOpener)) + if at == -1 { + return false + } + rest = rest[at+len(diffLineRecordOpener):] + + // The record reads ;;, and the kind is a single character. + digits := 0 + for digits < len(rest) && rest[digits] >= '0' && rest[digits] <= '9' { + digits++ + } + kind := digits + 1 + if digits == 0 || kind+1 >= len(rest) || rest[digits] != ';' || rest[kind+1] != ';' { + continue + } + if lo.Contains(diffLineRecordKinds, string(rest[kind:kind+1])) { + return true + } + } +} + +// diffStatEntry matches line against diffStatEntryPattern, behind the two checks that +// answer for nearly every line of a diff without the pattern being run at all: an entry +// of a diffstat is indented by a space, and holds the separator. The indices it returns +// are into the whole line, whatever the match was made past. +func diffStatEntry(line []byte) []int { + start := handshakeEnd(line) + rest := line[start:] + if len(rest) == 0 || rest[0] != ' ' || bytes.IndexByte(rest, '|') == -1 { + return nil + } + + match := diffStatEntryPattern.FindSubmatchIndex(rest) + for i := range match { + if match[i] >= 0 { + match[i] += start + } + } + return match +} + +// handshakeEnd returns where the record a renderer announces itself with ends, for a +// line that opens with one, and 0 for every other line. +// +// A renderer writes the handshake before anything else and with no newline after it, +// so it lands at the start of the first line of its output. For a diff with nothing +// above its diffstat — the diff of a range of commits, or of a stash — that is the line +// naming the first file in it, and the entry begins after the record rather than at the +// start of the line. +func handshakeEnd(line []byte) int { + if !bytes.HasPrefix(line, []byte(diffLineRecordOpener)) { + return 0 + } + + after := len(diffLineRecordOpener) + for after < len(line) && line[after] >= '0' && line[after] <= '9' { + after++ + } + + // Either terminator ends a record. One that goes on into a field instead is about + // a line of the diff, which is below the whole diffstat and no entry of it. + switch { + case after < len(line) && line[after] == '\x07': + return after + 1 + case after+1 < len(line) && line[after] == '\x1b' && line[after+1] == '\\': + return after + 2 + } + return 0 +} + +// JumpToFileNamedInDiffStat goes to the file of the pane's diff that the given diffstat +// entry names, for a click on the link made for that entry. It lands the way picking +// the file from the menu of the diff's files does. +// +// The diff is read to the end first, as it is for that menu. The diffstat is on screen +// only while the view is at the top of the diff, so the file clicked is nearly always +// below the part of it that has been read. +func (self *DiffLineHelper) JumpToFileNamedInDiffStat(pane types.DiffPaneContext, entry string) { + manager := self.c.GetViewBufferManagerForView(pane.GetView()) + if manager == nil { + return + } + manager.ReadToEnd(func() { + self.c.OnUIThread(func() error { + self.jumpToFileNamedInDiffStat(pane, entry) + return nil + }) + }) +} + +func (self *DiffLineHelper) jumpToFileNamedInDiffStat(pane types.DiffPaneContext, entry string) { + view := pane.GetView() + worktreePath := self.c.Git().RepoPaths.WorktreePath() + files := self.FilesInDiff(view) + names := lo.Map(files, func(file string, _ int) string { + return repoRelativePath(worktreePath, file) + }) + + index, ok := fileNamedByDiffStatEntry(entry, names) + if !ok { + self.c.ErrorToast(utils.ResolvePlaceholderString( + self.c.Tr.NoFileInDiffNamed, map[string]string{"path": entry})) + return + } + + if target, ok := self.StartOfFileInDiff(view, files[index]); ok { + self.PlaceNavigationTarget(pane, target, true) + } +} + +// fileNamedByDiffStatEntry returns which of the diff's files a diffstat entry names. +// +// An entry states the path as the diffstat has room for it rather than as git names +// the file. A path too long for the column is cut off on the left behind "...", and a +// rename is compacted to the "{old => new}" form. So the name is looked for among the +// files the diff turned out to hold, whole and then as the end of one, and is taken +// only where it names a single file. +func fileNamedByDiffStatEntry(entry string, paths []string) (int, bool) { + name := renamedTo(strings.TrimSpace(entry)) + + if index, ok := theOneMatching(paths, func(p string) bool { return p == name }); ok { + return index, true + } + + // Where the diffstat cut the path off, what is left is the end of it. The cut is at + // a directory boundary where there is room for one, and inside the file name where + // there isn't. + tail := strings.TrimPrefix(name, "...") + return theOneMatching(paths, func(p string) bool { return strings.HasSuffix(p, tail) }) +} + +// renamedTo returns the path a diffstat entry for a rename leaves the file at, and the +// entry itself for any other one. A rename states both paths, with whatever they have +// in common written once: "dir/{old => new}/file", or "old => new" where they share +// nothing. The part shared with the old path is gone along with the "{" when the entry +// is cut off on the left, which leaves a path to match the end of. +func renamedTo(entry string) string { + const arrow = " => " + at := strings.Index(entry, arrow) + if at == -1 { + return entry + } + + shared := "" + if brace := strings.Index(entry[:at], "{"); brace != -1 { + shared = entry[:brace] + } + renamed := entry[at+len(arrow):] + if closing := strings.Index(renamed, "}"); closing != -1 { + return shared + renamed[:closing] + renamed[closing+1:] + } + return shared + renamed +} + +// theOneMatching returns the index of the one element the predicate holds for, and +// false where it holds for none of them or for several. +func theOneMatching(paths []string, matches func(string) bool) (int, bool) { + found := -1 + for i, candidate := range paths { + if !matches(candidate) { + continue + } + if found != -1 { + return 0, false + } + found = i + } + return found, found != -1 +} diff --git a/pkg/gui/controllers/helpers/diff_stat_links_test.go b/pkg/gui/controllers/helpers/diff_stat_links_test.go new file mode 100644 index 000000000..5ee1bc447 --- /dev/null +++ b/pkg/gui/controllers/helpers/diff_stat_links_test.go @@ -0,0 +1,312 @@ +package helpers + +import ( + "bytes" + "fmt" + "slices" + "strings" + "testing" + + "github.com/jesseduffield/lazygit/pkg/gui/style" + "github.com/samber/lo" + "github.com/stretchr/testify/assert" +) + +// link is the line the writer is expected to produce for a diffstat entry: the space +// it is indented by, the name linked, and the rest of the line as it came. +func link(name string, rest string) string { + return " " + style.PrintHyperlink(name, DiffStatLinkScheme+name) + rest +} + +// record is the OSC 1717 record a diff renderer speaking the given version of the +// protocol states a line of the given kind with, as it precedes that line in its +// output. +func record(version string, kind string) string { + return fmt.Sprintf("%s%s;%s;;;pkg/gui.go\x1b\\", diffLineRecordOpener, version, kind) +} + +// handshake is the record a renderer announces the protocol with: the version it +// speaks, and nothing about any line. Renderers end their records with either +// terminator, so both turn up. +var ( + handshake = diffLineRecordOpener + "1\x1b\\" + handshakeBel = diffLineRecordOpener + "1\x07" +) + +func TestDiffStatLinkWriter(t *testing.T) { + scenarios := []struct { + name string + linkFiles bool + lines []string + expected []string + }{ + { + name: "links the entries of the diffstat, and nothing after it", + linkFiles: true, + lines: []string{ + "commit 1234567", + "", + " A commit message", + "", + " pkg/gui.go | 12 ++++++------", + " dir/other.go | 3 ++-", + " 2 files changed, 8 insertions(+), 7 deletions(-)", + "", + "diff --git a/pkg/gui.go b/pkg/gui.go", + " a context line that reads like an entry | 3 ++-", + }, + expected: []string{ + "commit 1234567", + "", + " A commit message", + "", + link("pkg/gui.go", " | 12 ++++++------"), + link("dir/other.go", " | 3 ++-"), + " 2 files changed, 8 insertions(+), 7 deletions(-)", + "", + "diff --git a/pkg/gui.go b/pkg/gui.go", + " a context line that reads like an entry | 3 ++-", + }, + }, + { + name: "links a binary file, a file that changes nothing, and a name with spaces", + linkFiles: true, + lines: []string{ + " logo.png | Bin 0 -> 1234 bytes", + " script.sh | 0", + " my file.txt | 2 +-", + }, + expected: []string{ + link("logo.png", " | Bin 0 -> 1234 bytes"), + link("script.sh", " | 0"), + link("my file.txt", " | 2 +-"), + }, + }, + { + name: "stops looking once the diff itself has begun", + linkFiles: true, + lines: []string{ + "diff --git a/pkg/gui.go b/pkg/gui.go", + " a context line that reads like an entry | 3 ++-", + }, + expected: []string{ + "diff --git a/pkg/gui.go b/pkg/gui.go", + " a context line that reads like an entry | 3 ++-", + }, + }, + { + name: "stops looking at the first line of the diff a renderer states", + linkFiles: true, + lines: []string{ + " pkg/gui.go | 1 +", + record("1", "c") + " a context line of the diff", + " this/looks/like/a/diff/stat | 2 +", + }, + expected: []string{ + link("pkg/gui.go", " | 1 +"), + record("1", "c") + " a context line of the diff", + " this/looks/like/a/diff/stat | 2 +", + }, + }, + { + // A renderer is free to state records about something that is no line of + // the diff, and one has stated the commit line above it. The diffstat is + // below such a record as much as it is below the handshake. + name: "keeps looking past a record of a kind it doesn't know", + linkFiles: true, + lines: []string{ + record("1", "C") + "commit 1234567", + " pkg/gui.go | 1 +", + }, + expected: []string{ + record("1", "C") + "commit 1234567", + link("pkg/gui.go", " | 1 +"), + }, + }, + { + name: "stops for a record of a protocol version it doesn't read", + linkFiles: true, + lines: []string{ + record("7", "f") + "── pkg/gui.go ──", + " this/looks/like/a/diff/stat | 2 +", + }, + expected: []string{ + record("7", "f") + "── pkg/gui.go ──", + " this/looks/like/a/diff/stat | 2 +", + }, + }, + { + name: "keeps looking past the handshake, which states no line", + linkFiles: true, + lines: []string{ + handshake, + " pkg/gui.go | 1 +", + }, + expected: []string{ + handshake, + link("pkg/gui.go", " | 1 +"), + }, + }, + { + // A diff with nothing above its diffstat. The handshake is written with no + // newline after it, so it runs into the entry naming the first file. + name: "links an entry the handshake runs into", + linkFiles: true, + lines: []string{ + handshake + " pkg/gui.go | 1 +", + " dir/other.go | 2 +-", + }, + expected: []string{ + handshake + link("pkg/gui.go", " | 1 +"), + link("dir/other.go", " | 2 +-"), + }, + }, + { + name: "links an entry a handshake ended with a BEL runs into", + linkFiles: true, + lines: []string{ + handshakeBel + " pkg/gui.go | 1 +", + }, + expected: []string{ + handshakeBel + link("pkg/gui.go", " | 1 +"), + }, + }, + { + name: "leaves a render whose file names lead nowhere alone", + linkFiles: false, + lines: []string{ + " pkg/gui.go | 12 ++++++------", + }, + expected: []string{ + " pkg/gui.go | 12 ++++++------", + }, + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + buffer := &bytes.Buffer{} + writer := NewDiffStatLinkWriter(buffer) + writer.BeginRender(scenario.linkFiles) + + for _, line := range scenario.lines { + written, err := writer.Write([]byte(line + "\n")) + assert.NoError(t, err) + // The writer answers for what it was given, not for what it passed on. + assert.Equal(t, len(line)+1, written) + } + + assert.Equal(t, strings.Join(scenario.expected, "\n")+"\n", buffer.String()) + }) + } +} + +func TestDiffStatLinkWriterStartsLookingAgainWithEachRender(t *testing.T) { + buffer := &bytes.Buffer{} + writer := NewDiffStatLinkWriter(buffer) + + for range 2 { + buffer.Reset() + writer.BeginRender(true) + _, _ = writer.Write([]byte(" pkg/gui.go | 1 +\n")) + _, _ = writer.Write([]byte(" 1 file changed, 1 insertion(+)\n")) + + assert.Equal(t, link("pkg/gui.go", " | 1 +")+ + "\n 1 file changed, 1 insertion(+)\n", buffer.String()) + } +} + +func TestFileNamedByDiffStatEntry(t *testing.T) { + paths := []string{ + "pkg/gui.go", + "pkg/integration/tests/main_view/jump_to_a_file_of_the_diff.go", + "vendor/github.com/gdamore/tcell/v3/AUTHORS", + "pkg/gocui/AUTHORS", + "renamed.txt", + "a/very/deeply/nested/directory/structure/some_long_file_name.txt", + } + + scenarios := []struct { + name string + entry string + expected string + }{ + { + name: "a path the diffstat had room for", + entry: "pkg/gui.go", + expected: "pkg/gui.go", + }, + { + name: "a path cut off at a directory boundary", + entry: ".../tests/main_view/jump_to_a_file_of_the_diff.go", + expected: "pkg/integration/tests/main_view/jump_to_a_file_of_the_diff.go", + }, + { + name: "a path cut off inside the file name", + entry: "..._long_file_name.txt", + expected: "a/very/deeply/nested/directory/structure/some_long_file_name.txt", + }, + { + name: "a rename, stated as the part the two paths share", + entry: "vendor/github.com/gdamore/tcell/{v2 => v3}/AUTHORS", + expected: "vendor/github.com/gdamore/tcell/v3/AUTHORS", + }, + { + name: "a rename whose shared part was cut off along with the brace", + entry: ".../github.com/jesseduffield => pkg}/gocui/AUTHORS", + expected: "pkg/gocui/AUTHORS", + }, + { + name: "a rename of paths that share nothing", + entry: "original.txt => renamed.txt", + expected: "renamed.txt", + }, + { + name: "a name of no file of the diff", + entry: "pkg/nowhere.go", + expected: "", + }, + { + name: "a name several files of the diff end with", + entry: "AUTHORS", + expected: "", + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + index, ok := fileNamedByDiffStatEntry(scenario.entry, paths) + if scenario.expected == "" { + assert.False(t, ok) + return + } + assert.True(t, ok) + assert.Equal(t, scenario.expected, paths[index]) + }) + } +} + +// TestEveryRecordKindIsWeighedAgainstTheDiffStat fails when the protocol gains a kind +// of record that nobody has placed relative to the diffstat. Where a line stating that +// kind can only come below the diffstat, it ends the search for it and belongs in +// diffLineRecordKinds; where it can come above the diff — a record about the commit, +// say — it says nothing about where the diffstat ends, and belongs in the list here. +// +// The parser takes the kind as a field rather than a table, so the kinds it reads are +// found by asking it about each character in turn. +func TestEveryRecordKindIsWeighedAgainstTheDiffStat(t *testing.T) { + kindsAboveTheDiff := []string{} + + printable := lo.RangeFrom(byte(' '), 0x7f-' ') + kindsTheParserReads := lo.FilterMap(printable, func(char byte, _ int) (string, bool) { + kind := string([]byte{char}) + _, ok := diffLineTypeFromMetadata(kind) + return kind, ok + }) + + assert.ElementsMatch(t, + append(slices.Clone(diffLineRecordKinds), kindsAboveTheDiff...), + kindsTheParserReads, + "a kind of record has been added to the protocol without being weighed "+ + "against the diffstat; see this test's comment for where it belongs") +} diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 0292670a0..32fd7718e 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -82,6 +82,9 @@ type Gui struct { statusManager *status.StatusManager waitForIntro sync.WaitGroup viewBufferManagerMap map[string]*tasks.ViewBufferManager + // holds a mapping of the main section's view names to the writers that link the + // files named in the diffstat of what is rendered into them + diffStatLinkWriterMap map[string]*helpers.DiffStatLinkWriter // holds a mapping of view names to ptmx's. This is for rendering command outputs // from within a pty. The point of keeping track of them is so that if we re-size // the window, we can tell the pty it needs to resize accordingly. @@ -415,6 +418,17 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context return gui.helpers.Files.EditFiles([]string{filepath}) } + if entry, ok := strings.CutPrefix(url, helpers.DiffStatLinkScheme); ok { + view, err := gui.g.View(viewname) + if err != nil { + return nil + } + if pane := gui.mainContextForView(view); pane != nil { + gui.helpers.DiffLine.JumpToFileNamedInDiffStat(pane, entry) + } + return nil + } + if err := gui.os.OpenLink(url); err != nil { return fmt.Errorf(gui.Tr.FailedToOpenURL, url, err) } @@ -775,17 +789,18 @@ func NewGui( test integrationTypes.IntegrationTest, ) (*Gui, error) { gui := &Gui{ - Common: cmn, - gitVersion: gitVersion, - Config: configurer, - Updater: updater, - statusManager: status.NewStatusManager(), - viewBufferManagerMap: map[string]*tasks.ViewBufferManager{}, - viewPtmxMap: map[string]oscommands.Pty{}, - showRecentRepos: showRecentRepos, - RepoPathStack: &utils.Stack[types.RepoLocation]{}, - RepoStateMap: map[Repo]*GuiRepoState{}, - GuiLog: []string{}, + Common: cmn, + gitVersion: gitVersion, + Config: configurer, + Updater: updater, + statusManager: status.NewStatusManager(), + viewBufferManagerMap: map[string]*tasks.ViewBufferManager{}, + diffStatLinkWriterMap: map[string]*helpers.DiffStatLinkWriter{}, + viewPtmxMap: map[string]oscommands.Pty{}, + showRecentRepos: showRecentRepos, + RepoPathStack: &utils.Stack[types.RepoLocation]{}, + RepoStateMap: map[Repo]*GuiRepoState{}, + GuiLog: []string{}, // initializing this to true for the time being; it will be reset to the // real value after loading the user config: diff --git a/pkg/gui/main_panels.go b/pkg/gui/main_panels.go index dbad48cbb..5a723125b 100644 --- a/pkg/gui/main_panels.go +++ b/pkg/gui/main_panels.go @@ -74,6 +74,12 @@ func (gui *Gui) RefreshMainView(opts *types.ViewUpdateOpts, context types.Contex // or a log, and reads as badly cut off at the edge of the pane as it would // anywhere else. view.Wrap = !mainContext.ContentIsDiff() || gui.c.UserConfig().Gui.WrapLinesInDiffView + // The files named in the diffstat are linked to where their diff begins, over a + // render that has both: the panel's own diff, and rows that can be placed in the + // files they show. The writer is told here, on the UI thread, since it is asked + // on the one reading the command's output. + gui.diffStatLinkWriter(view).BeginRender( + mainContext.ContentIsDiff() && gui.helpers.DiffLine.DiffRowsCanBePlaced()) } if err := gui.runTaskForView(view, opts.Task); err != nil { diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index e2d69eaf7..644b642a9 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/tasks" "github.com/sirupsen/logrus" ) @@ -156,12 +157,34 @@ func (gui *Gui) newStringTaskWithKey(view *gocui.View, str string, key string) e return nil } +// contentWriter returns what a render of the given view writes its content to: the +// view itself, or, for a pane of the main section, the writer that links the files +// named in the diffstat on its way there (see DiffStatLinkWriter). +func (gui *Gui) contentWriter(view *gocui.View) io.Writer { + if gui.mainContextForView(view) == nil { + return view + } + return gui.diffStatLinkWriter(view) +} + +// diffStatLinkWriter returns the writer that links the diffstat of the given pane, +// making it if the pane hasn't rendered yet. It lasts as long as the view does, and +// each render tells it what to make of that render (see DiffStatLinkWriter.BeginRender). +func (gui *Gui) diffStatLinkWriter(view *gocui.View) *helpers.DiffStatLinkWriter { + writer, ok := gui.diffStatLinkWriterMap[view.Name()] + if !ok { + writer = helpers.NewDiffStatLinkWriter(view) + gui.diffStatLinkWriterMap[view.Name()] = writer + } + return writer +} + func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager { manager, ok := gui.viewBufferManagerMap[view.Name()] if !ok { manager = tasks.NewViewBufferManager( gui.Log, - view, + gui.contentWriter(view), func() { // Called before showing the "loading..." indicator: clear the // displayed buffer so only "loading..." is shown. The actual content diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 4aee3fb42..efd84d722 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -421,6 +421,7 @@ type TranslationSet struct { JumpToFileInDiff string JumpToFileInDiffTooltip string OnlyOneFileInDiff string + NoFileInDiffNamed string PrevConflict string NextConflict string SelectPrevHunk string @@ -1612,6 +1613,7 @@ func EnglishTranslationSet() *TranslationSet { JumpToFileInDiff: "Jump to file in diff", JumpToFileInDiffTooltip: "Pick one of the files of the diff shown in the main view, and scroll the main view to it. The focus stays in this panel.", OnlyOneFileInDiff: "There is only one file in this diff", + NoFileInDiffNamed: "This diff has no file named '{{.path}}'", PrevConflict: "Previous conflict", NextConflict: "Next conflict", SelectPrevHunk: "Previous hunk", diff --git a/pkg/integration/tests/main_view/click_a_file_in_a_diff_stat_that_comes_first.go b/pkg/integration/tests/main_view/click_a_file_in_a_diff_stat_that_comes_first.go new file mode 100644 index 000000000..2d459e21d --- /dev/null +++ b/pkg/integration/tests/main_view/click_a_file_in_a_diff_stat_that_comes_first.go @@ -0,0 +1,58 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ClickAFileInADiffStatThatComesFirst = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Jump to a file by clicking its name in a diffstat the renderer's handshake runs into", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 100, + Height: 30, + SetupConfig: func(cfg *config.AppConfig) { + // A renderer that announces the protocol and then passes the diff on as it came. + // The handshake has no newline after it, so it runs into the first line the + // renderer is given — which for the diff of a range of commits is the first + // entry of the diffstat, there being no commit above it. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Command: `printf '\033]1717;1\007'; cat`}, + } + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("aaa.txt", "one\n") + shell.Commit("one") + shell.CreateFileAndAdd("zzz.txt", "one\n") + shell.Commit("two") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + SelectedLine(Contains("two")). + Press(keys.Universal.ToggleRangeSelect). + SelectNextItem(). + SelectedLines( + Contains("two"), + Contains("one"), + ) + + // Below the line lazygit writes to say what the diff is of, the diff opens with + // the diffstat, and the handshake runs into its first entry. That is the one + // clicked here. + t.Views().Main(). + TopLines( + Contains("Showing diff for range"), + Equals(""), + Contains("aaa.txt"), + Contains("zzz.txt"), + Contains("2 files changed"), + ). + Click(2, 2) + + t.Views().Commits().IsFocused() + + t.Views().Main(). + TopVisibleLine(Contains("diff --git a/aaa.txt b/aaa.txt")) + }, +}) diff --git a/pkg/integration/tests/main_view/click_a_file_in_the_diff_stat.go b/pkg/integration/tests/main_view/click_a_file_in_the_diff_stat.go new file mode 100644 index 000000000..1c28277f0 --- /dev/null +++ b/pkg/integration/tests/main_view/click_a_file_in_the_diff_stat.go @@ -0,0 +1,90 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ClickAFileInTheDiffStat = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Jump to a file of a commit's diff by clicking the line that names it in the diffstat", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 100, + Height: 30, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + lines := make([]string, 600) + for i := range lines { + lines[i] = fmt.Sprintf("line%03d", i+1) + } + // A long file at either end, so that the file jumped to is far below the + // diffstat and has a diff under it to scroll past. + shell.CreateFileAndAdd("aaa.txt", strings.Join(lines, "\n")+"\n") + shell.CreateFileAndAdd("dir/bbb.txt", "one\n") + shell.CreateFileAndAdd("zzz.txt", strings.Join(lines, "\n")+"\n") + shell.Commit("one") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + SelectedLine(Contains("one")) + + // The click below is at a line of the diffstat, so the diff has to open with + // the lines this expects. + t.Views().Main(). + TopLines( + Contains("commit"), + Contains("Author:"), + Contains("Date:"), + Equals(""), + Contains("one"), + Equals("---"), + Contains("aaa.txt"), + Contains("dir/bbb.txt"), + Contains("zzz.txt"), + Contains("3 files changed"), + ). + Click(2, 8) + + // The panel keeps the focus, and the diff goes to the file clicked. + t.Views().Commits(). + IsFocused(). + SelectedLine(Contains("one")) + + t.Views().Main(). + TopVisibleLine(Contains("diff --git a/zzz.txt b/zzz.txt")) + + t.Views().Commits(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + // With the pane focused there is a selection to move, and the click moves it to + // the file, exactly as picking the file from the menu would. + t.Views().Main(). + IsFocused(). + SelectionIsActive(). + Press(keys.Universal.GotoTop) + + // The first file's diff begins on screen already, so the view stays where it + // is; a jump only scrolls as far as it must once there is a selection to point + // at the file with. + t.Views().Main(). + TopVisibleLine(Contains("commit")). + Click(2, 6). + SelectedLines( + Contains("diff --git a/aaa.txt b/aaa.txt"), + ). + TopVisibleLine(Contains("commit")). + // The last file's is far below, so that one is scrolled to. + Click(2, 8). + SelectedLines( + Contains("diff --git a/zzz.txt b/zzz.txt"), + ). + TopVisibleLine(Contains("diff --git a/zzz.txt b/zzz.txt")) + }, +}) diff --git a/pkg/integration/tests/main_view/no_diff_stat_links_under_an_external_diff.go b/pkg/integration/tests/main_view/no_diff_stat_links_under_an_external_diff.go new file mode 100644 index 000000000..8edee63cc --- /dev/null +++ b/pkg/integration/tests/main_view/no_diff_stat_links_under_an_external_diff.go @@ -0,0 +1,54 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var NoDiffStatLinksUnderAnExternalDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The files named in the diffstat are not linked under a diff renderer that says nothing about its rows", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 100, + Height: 30, + SetupConfig: func(cfg *config.AppConfig) { + // An external diff whose output has nothing to say about which line of which + // file each row shows. git writes the diffstat itself, so the names are there + // to be clicked, but nothing could find the file they name. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Name: "opaque", Type: "extDiff", Command: `sh -c 'echo EXT'`}, + } + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("aaa.txt", "one\n") + shell.CreateFileAndAdd("zzz.txt", "one\n") + shell.Commit("one") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + SelectedLine(Contains("one")) + + // The click below is at a line of the diffstat, so the diff has to open with + // the lines this expects. + t.Views().Main(). + TopLines( + Contains("commit"), + Contains("Author:"), + Contains("Date:"), + Equals(""), + Contains("one"), + Equals("---"), + Contains("aaa.txt"), + Contains("zzz.txt"), + Contains("2 files changed"), + ). + Click(2, 7) + + // The name is no link, so the click is an ordinary one, which focuses the pane + // it lands in. Were it a link, it would have been followed instead, and would + // have had to report that it found no such file — the test fails on the toast + // that leaves unacknowledged. + t.Views().Main().IsFocused() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index dad72fd68..264a4bc95 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -378,6 +378,8 @@ var tests = []*components.IntegrationTest{ main_view.BuildPatchWithMixedSelections, main_view.ChangeContextSizeWhileBuildingPatch, main_view.ChangeScreenModeInFocusedDiff, + main_view.ClickAFileInADiffStatThatComesFirst, + main_view.ClickAFileInTheDiffStat, main_view.ClickSelectsDiffLine, main_view.CommitFromMainView, main_view.CopyRowsThatAreNoDiffLine, @@ -442,6 +444,7 @@ var tests = []*components.IntegrationTest{ main_view.MovePatchToNewCommitBefore, main_view.MovePatchToNewCommitInStackedBranch, main_view.NavigateByHunkAndFile, + main_view.NoDiffStatLinksUnderAnExternalDiff, main_view.NoSelectionOverABinaryDiff, main_view.NoSelectionOverACommitLog, main_view.NoSelectionOverAConflictHint,