Unfortunately it isn't possible to delete them. This would often be useful, but
our todo rewriting mechanisms rely on being able to find todos by some
identifier (hash for pick, ref for update-ref), and exec todos don't have a
unique identifier.
If exactly one candidate from inside the current branch is found, we return that
one even if there are also hunks belonging to master commits; we disregard those
in this case.
It has two modified hunks, one for a master commit and one for a branch commit.
Currently we get an error mentioning those two commits, but we would like to
silently select the branch commit.
Put it into the individual menu items instead.
Again, this is necessary because we are going to add another entry to the menu
that is independent of the selected branch.
Previously the entire status was colored in a single color, so the API made
sense. This is going to change in the next commit, so now we must include the
color in the string returned from BranchStatus(), which means that callers who
need to do hit detection or measure the length need to decolorize it.
While we're at it, switch the order of ↑3↓7 to ↓7↑3. For some reason that I
can't really explain I find it more logical this way. The software out there is
pretty undecided about it, it seems: VS Code puts ↓7 first, and so does the
shell prompt that comes with git; git status and git branch -v put "ahead" first
though. Shrug.
Use Equals instead of Contains for asserting the status view content. This
solves the problem that we might assert Contains("↓2 repo"), but what it really
shows is "↑1↓2 repo", and the test still succeeds. At best this is confusing.
Also, this way we don't have to use the awkward DoesNotContain to check that it
really doesn't show a checkmark.
To do this, we need to fix two whitespace problems:
- there was always a space at the end for no reason. Simply remove it. It was
added in efb51eee96, but from looking at that diff it seems it was added
accidentally.
- there was a space at the beginning if the branch status was empty. This is
actually a cosmetic problem, for branches without a status the text was
indented by once space. Change this so that the space is added conditionally.
It's a bit awkward that we have to use Decolorise here, but this will go away
again later in this branch.
It can optionally be used to set the title of the panel that shows the output of
a command (when showOutput is true). If left unset, the command string is used
as the title.
To determine whether we need to ask for force pushing, we need to query the push
branch rather than the upstream branch, in case they are not the same.
Our code doesn't realize that we need to prompt the user to force push, when the
branch is up-to-date with its upstream but not with the branch that we're
pushing to.
It is unexpected that a function called PushBranch also sets the upstream
branch; also, we want to add a PushBranch function in the next commit that
doesn't.
In go 1.22, loop variables are redeclared with each iteration of the
loop, rather than simple updated on each iteration. This means that we
no longer need to manually redeclare variables when they're closed over
by a function.
For custom commands it is useful to select an earlier command and have it copied
to the prompt for further editing. This can be done by hitting 'e' now.
For other types of suggestion panels we don't enable this behavior, as you can't
create arbitrary new items there that don't already exist as a suggestion.
In the custom commands panel you can now tab to the suggestions and hit 'd' to
delete items from there. Useful if you mistyped a command and don't want it to
appear in your history any more.
The rebase.updateRefs feature of git is very useful to rebase a stack of
branches and keep everything nicely stacked; however, it is usually in the way
when you make a copy of a branch and want to rebase it "away" from the original
branch in some way or other. For example, the original branch might sit on main,
and you want to rebase the copy onto devel to see if things still compile there.
Or you want to do some heavy history rewriting experiments on the copy, but keep
the original branch in case the experiments fail. Or you want to split a branch
in two because it contains two unrelated sets of changes; so you make a copy,
and drop half of the commits from the copy, then check out the original branch
and drop the other half of the commits from it.
In all these cases, git's updateRefs feature insists on moving the original
branch along with the copy in the first rebase that you make on the copy. I
think this is a bug in git, it should create update-ref todos only for branches
that point into the middle of your branch (because only then do they form a
stack), not when they point at the head (because then it's a copy). I had a long
discussion about this on the git mailing list [1], but people either don't agree
or don't care enough.
So we fix this on our side: whenever we start a rebase for whatever reason, be
it interactive, non-interactive, or behind-the-scenes, we drop any update-ref
todos that are at the very top of the todo list, which fixes all the
above-mentioned scenarios nicely.
I will admit that there's one scenario where git's behavior is the desired one,
and the fix in this PR makes it worse: when you create a new branch off of an
existing one, with the intention of creating a stack of branches, but before you
make the first commit on the new branch you realize some problem with the first
branch (e.g. a commit that needs to be reworded or dropped). It this case you do
want both branches to be affected by the change. In my experience this scenario
is much rarer than the other ones that I described above, and it's also much
easier to recover from: just check out the other branch again and hard-reset it
to the rebased one.
[1]
https://public-inbox.org/git/354f9fed-567f-42c8-9da9-148a5e223022@haller-berlin.de/
We upgraded our minimum Go version to 1.21 in commit
57ac9c2189. We can now replace our
`utils.Min` and `utils.Max` functions with the built-in `min` and `max`.
Reference: https://go.dev/ref/spec#Min_and_max
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Using the "Add to .git/info/exclude" in a worktree results in an error message,
as the test shows. The same would happen in a submodule, but I'm not adding an
extra test for that, as the circumstances are the same.
When exiting filtering mode, we currently keep the selection index the same in
the commits panel. This doesn't make sense at all, since the index in the
filtered view has no relation to the index in the unfiltered view.
I often use filtering mode (either by path or by author) to find a given commit
faster than I would otherwise be able to. When exiting filtering mode, it's
useful to keep the same commit selected, so that I can look at the surrounding
commits, see which branch it was a part of, etc. So reselect the commit again
after exiting filtering mode.
Sometimes this is not possible, most likely when the commit is so long ago that
it's outside of the initial 300 range. In that case, at least select the commit
again that was selected before I entered filtering; this is still better than
arbitrarily keeping the same selection index.
It would crash when some keybindings are set to null, and the filter string is
such that only those keybindings remain visible.
The reason for the crash is that when inserting non-model items (menu section
headers in this case) you specify a column to align them to. This works on the
assumption that the number of columns is always the same. It can cope with the
case that columns are removed because they are empty for all items; but it can't
cope with the case that the getDisplayStrings function returns a lower number of
columns.
And this is what happened here: MenuViewModel.GetDisplayStrings would omit the
keybinding column when none of the entries have a keybinding. This logic is
unnecessary, the generic list rendering mechanism takes care of this, so
removing this logic fixes the crash.
We do have to make sure though that the column is really empty when there's no
keybinding, so change the logic to use FgCyan only when there's a keybinding.
It is a bad idea to read a git-rebase-todo file, remove some update-ref todos,
and write it back out behind git's back. This will cause git to actually remove
the branches referenced by those update-ref todos when the rebase is continued.
The reason is that git remembers the refs affected by update-ref todos at the
beginning of the rebase, and remembers information about them in the file
.git/rebase-merge/update-refs. Then, whenever the user performs a "git rebase
--edit-todo" command, it updates that file based on whether update-ref todos
were added or removed by that edit. If we rewrite the git-rebase-todo file
behind git's back, this updating doesn't happen.
Fix this by not updating the git-rebase-todo file directly in this case, but
performing a "git rebase --edit-todo" command where we set ourselves as the
editor and change the file in there. This makes git update the bookkeeping
information properly.
Ideally we would use this method for all cases where we change the
git-rebase-todo file (e.g. moving todos up/down, or changing the type of a
todo); this would be cleaner because we wouldn't mess with git's private
implementation details. I tried this, but unfortunately it isn't fast enough.
Right now, moving a todo up or down takes between 1 and 2ms on my machine;
changing it to do a "git rebase --edit-todo" slows it down to over 100ms, which
is unacceptable.
In the test we simply removed the update-ref todo but didn't make any other
changes to the todos. This should really have kept everything the way it was,
including the other branch head. The fact that the star was gone was really
because of the bug that we are going to fix later in the branch.
Change the test so that it also makes a change before the update-ref todo; this
way we test that the star is gone because we deleted the update-ref, not because
of the bug.
To guard against the bug, we add another assertion for the branches view to test
that both branches are still there. This currently fails.
To support this, we turn the confirmation prompt of the "Create fixup commit"
command into a menu; creating a fixup commit is the first entry, so that
"shift-F, enter" behaves the same as before. But there are additional entries
for creating "amend!" commits, either with or without file changes. These make
it easy to reword commit messages of existing commits.
By default we now search for substrings; you can search for multiple substrings
by separating them with spaces. Add a config option gui.filterMode that can be
set to 'fuzzy' to switch back to the previous behavior.
We have such a test already (squash_fixups_above_first_commit.go), but it can't
be used for what we want to check here, because it uses the first commit, and we
can't move down from there. So create a new one that basically does the same
thing, but for a commit in the middle. The focus of this new test is to check
how the selection behaves; as you can see, there is a problem both when creating
a fixup and when squashing fixups. We'll address these separately in the next
commits.
It starts a rebase on the bottom-most commit of the range, and sets all the
selected commits to "edit" (skipping merge commits, because they can't be
edited).
In most real-world scenarios, name and path are usually the same. They don't
have to be though, and it's important to make sure we use the right one when
passing arguments to git commands, so change the tests to have different name
and path.
The additional branch head icon is more confusing than useful in this situation.
The update-ref entries show very clearly where the branch heads will go when
continuing the rebase; the information where the branch heads used to be before
the rebase is not really needed here, and just makes the display more confusing.
I'm not adding more tests here because the changes to the existing tests
demonstrate the change clearly enough.
This commit introduces a new feature to the commit view, allowing users
to filter commits based on the author's name or email address. Similar
to the existing path filtering functionality, accessible through <c-s>,
this feature allows users to filter the commit history by the currently
selected commit's author if the commit view is focused, or by typing in
the author's name or email address.
This feature adds an entry to the filtering menu, to provide users with
a familiar and intuitive experience
For some lists it is useful to keep the same sort order when filtering (rather
than sorting by best match like we usually do). Add an optional function to
FilteredList to make this possible, and use it whenever we show lists of things
sorted by date (branches, stashes, reflog entries), as well as menu items
because this allows us to keep the section headers in the keybindings menu,
which is useful for understanding what you are looking at when filtering.
This fixes two problems:
- each time the custom commands panel was opened, the history of commands would
be shown in reversed order compared to last time. (The reason is that
lo.Reverse modifies the slice in place rather than just returning a new,
reversed slice.)
- when executing a previous command again (either by typing it in again, or by
picking it from the history), it should move to the beginning of the history,
but didn't.
We fix this by storing the history in reversed order (as the user sees it in
the panel), this makes the logic simpler. We just have to prepend rather
than append newly added commands now.
While this is theoretically a breaking change, it's not worth bothering because
the order was wrong for existing users in 50% of the cases anyway.
After discarding file changes from the commit, the was still referencing
these indexes as being part of the range select. The consequence was
needing to hit escape twice to exit commit files in some situations.
Canceling the range select after discarding changes fixes that.
I'm combining the delete single file case from `discard_old_file_change`
with the content of `discard_range_select` and calling that
`discard_old_file_changes`. Hopefully that cleans things up a little
bit.
This also adds a check that the custom patch is getting reset properly.
It can be tedious after each cherry-pick opearation to clear the
selection by pressing escape in order for lazygit to stop displaying
info about copied commits. Also, it seems to be a rare case to
cherry-pick commits to more than one destination.
The simplest solution to address this issue is to clear the selection
upon paste.
The only exception is a merge conflict. Initially, I wanted to clear
selected commits in this scenario too. During a discussion we found out
that it may be convenient to have the copied commits still around.
Aborting the rebase and pasting the commits in the middle of a branch
can be a valid use case.
To do that, change the "Apply fixup commits" command to show a menu with the two
choices "in current branch" and "above the selected commit"; we make "in current
branch" the default, as it's the more useful one most of the time, even though
it is a breaking change for those who are used to "shift-S enter" meaning
"squash above selected".
As part of making lazygit more discoverable, there are certain keys which you almost certainly
need to press when you're in a given mode e.g. 'v' to paste commits when cherry-picking. This
commit prominently shows these keybinding suggestions alongside the others in the option view.
I'm using the same colours for these keybindings as is associated with the mode elsewhere e.g.
yellow for rebasing and cyan for cherry-picking. The cherry-picking one is a bit weird because
we also use cyan text to show loaders and app status at the bottom left so it may be confusing,
but I haven't personally found it awkward from having tested it out myself.
Previously we would render these options whenever a new context was activated, but now that we
need to re-render options whenever a mode changes, I'm instead rendering them on each screen
re-render (i.e. in the layout function). Given how cheap it is to render this text, I think
it's fine performance-wise.
This adds a bunch of tooltips to keybindings and updates some keybinding descriptions (i.e. labels).
It's in preparation for displaying more keybindings on-screen (in the bottom right of the screen),
and so due in part to laziness it shortens some descriptions so that we don't need to manage both
a short and long description (for on-screen vs in-menu). Nonetheless I've added a ShortDescription
field for when we do want to have both a short and long description.
You'll notice that some keybindings I deemed unworthy of the options view have longer descriptions,
because I could get away with it.
This is useful if you want to move a range of commits, so you select them, and
then realize it's better to do it in an interactive rebase. Pressing 'i'
preserves the range now.
As part of this, you must now press enter on a merge conflict file
to focus the merge view; you can no longer press space and if you do
it will raise an error.
This changes GetRepoPaths() to pull information from `git rev-parse`
instead of effectively reimplementing git's logic for pathfinding. This
change fixes issues with bare repos, esp. versioned homedir use cases,
by aligning lazygit's path handling to what git itself does.
This change also enables lazygit to run from arbitrary subdirectories of
a repository, including correct handling of symlinks, including "deep"
symlinks into a repo, worktree, a repo's submodules, etc.
Integration tests are now resilient against unintended side effects from
the host's environment variables. Of necessity, $PATH and $TERM are the
only env vars allowed through now.
Previously if we marked a line with IsSelected() we would check if it was selected, but
we would not check if other lines were unexpectedly selected. Now, if you use IsSelected(),
we ensure that _only_ the lines you marked as such are the selected lines.
This wasn't necessary before, because the only available branch sorting option
was by recency, so the sort order couldn't change except by checking out
branches. Now, you can sort by committer date, so the branch order can change by
fetching; in this case it's important to keep the same branch selected. One
important use case is to rebase the checked-out branch onto master; you select
master, press "f" to fetch it (this can now change its position in the list),
and then press "r" to rebase. To make this work smoothly it's important to keep
master selected after pressing "f".
We want to show an error when the user tries to invoke an action that expects only
a single item to be selected.
We're using the GetDisabledReason field to enforce this (as well as DisabledReason
on menu items).
I've created a ListControllerTrait to store some shared convenience functions for this.
This requires us to change the 'v' keybinding for paste to something else,
now that 'v' is used globally for toggling range select. So I'm using
'shift+v' and I'm likewise changing 'c' to 'shift+c' for copying, so
that they're consistent.
We will need to clearly communicate this change in keybindings.
This is the highest priority of the escape actions because it's the thing you're
most likely to want to do upon hitting escape if you have a range selected.
Applying this to the staging/patch-building views is tricky: if we want this logic
for when a range of lines is selected, we'll also need to apply it when a hunk
is selected too. I still think it's worth it though: I've often accidentally
escaped from the staging view when trying to cancel a range selection.
We're not fully standardising here: different contexts can store their range state however
they like. What we are standardising on is that now the view is always responsible for
highlighting the selected lines, meaning the context/controller needs to tell the view
where the range start is.
Two convenient benefits from this change:
1) we no longer need bespoke code in integration tests for asserting on selected lines because
we can just ask the view
2) line selection in staging/patch-building/merge-conflicts views now look the same as in
list views i.e. the highlight applies to the whole line (including trailing space)
I also noticed a bug with merge conflicts not rendering the selection on focus though I suspect
it wasn't a bug with any real consequences when the view wasn't displaying the selection.
I'm going to scrap the selectedRangeBgColor config and just let it use the single line
background color. Hopefully nobody cares, but there's really no need for an extra config.
This adds range select ability in two ways:
1) Sticky: like what we already have with the staging view i.e. press v then use arrow keys
2) Non-sticky: where you just use shift+up/down to expand the range
The state machine works like this:
(no range, press 'v') -> sticky range
(no range, press arrow) -> no range
(no range, press shift+arrow) -> nonsticky range
(sticky range, press 'v') -> no range
(sticky range, press arrow) -> sticky range
(sticky range, press shift+arrow) -> nonsticky range
(nonsticky range, press 'v') -> no range
(nonsticky range, press arrow) -> no range
(nonsticky range, press shift+arrow) -> nonsticky range
A common issue I have is that I want to move a commit from the top of my branch
all the way down to the first commit on the branch. To do that, I need to navigate
down to the first commit on my branch, press 'e' to start an interactive rebase,
then navigate back up to the top of the branch, then move my commit back down to
the base. This is annoying.
Similarly annoying is moving the commit one-by-one without explicitly starting
an interactive rebase, because then each individual step is its own rebase which
takes a while in aggregate.
This PR allows you to press 'i' from the commits view to start an interactive
rebase from an 'appropriate' base. By appropriate, we mean that we want to start
from the HEAD and stop when we reach the first merge commit or commit on the main
branch. This may end up including more commits than you need, but it doesn't make
a difference.
For some bizarre reason `pkg/integration/tests/filter_by_path/cli_arg.go` is failing as of 8c716184 like so:
```
test_lazygit
Usage:
test_lazygit [git-arg]
Positional Variables:
git-arg Panel to focus upon opening lazygit. Accepted values (based on git terminology): status, branch, log, stash. Ignored if --filter arg is passed.
Flags:
-h --help Displays help with available flag, subcommand, and positional value parameters.
-p --path Path of git repo. (equivalent to --work-tree=<path> --git-dir=<path>/.git/)
-f --filter Path to filter on in `git log -- <path>`. When in filter mode, the commits, reflog, and stash are filtered based on the given path, and some operations are restricted
-v --version Print the current version
-d --debug Run in debug mode with logging (see --logs flag below). Use the LOG_LEVEL env var to set the log level (debug/info/warn/error) (default: false)
-l --logs Tail lazygit logs (intended to be used when `lazygit --debug` is called in a separate terminal tab)
-c --config Print the default config
-cd --print-config-dir Print the config directory
-ucd --use-config-dir override default config directory with provided directory
-w --work-tree equivalent of the --work-tree git argument
-g --git-dir equivalent of the --git-dir git argument
-ucf --use-config-file Comma separated list to custom config file(s)
Unknown arguments supplied: filterFile
```
where the CLI args are:
```
([]string) (len=5 cap=5) {
(string) (len=25) "/tmp/lazygit/test_lazygit",
(string) (len=6) "-debug",
(string) (len=108) "--use-config-dir=/Users/jesseduffieldduffield/repos/lazygit/test/_results/filter_by_path/cli_arg/used_config",
(string) (len=2) "-f",
(string) (len=10) "filterFile"
}
```
This appears to be a bug in flaggy itself. I've updated to the latest version but it still breaks. Bizarrely it works fine on CI and
only fails locally. Running lazygit locally with `lg -f pkg/gui/controllers/helpers/refresh_helper.go` it works fine. So I don't
know what's going on there. At any rate, I'm just going to get the test passing by passing `-f=filterFile` as a single argument.
We need to fetch our list of tests both outside of our test binary and within. We need
to get the list from within so that we can run the code that drives the test and runs
assertions. To get the list of tests we need to know where the root of the lazygit repo
is, given that the tests live in files under that root.
So far, we've used this GetLazyRootDirectory() function for that, but it assumes that
we're not in a test directory (it just looks for the first .git dir it can find). Because
we didn't want to properly fix this before, we've been setting the working directory of
the test command to the lazygit root, and using the --path CLI arg to override it when
the test itself ran. This was a terrible hack.
Now, we're passing the lazygit root directory as an env var to the integration test, so
that we can set the working directory to the actual path of the test repo; removing the
need to use the --path arg.
The algorithm works by blaming the deleted lines, so if a hunk contains only
added lines, we can only hope that it also belongs in the same commit. Warn the
user about this.
Note: the warning might be overly agressive, we'll have to see if this is
annoying. The reason is that it depends on the diff context size whether added
lines go into their own hunk or are grouped together with other added or deleted
lines into one hunk. However, our algorithm uses a diff context size of 0,
because that makes it easiest to parse the diff; this results in hunks having
only added lines more often than what the user sees. For example, moving a line
of code down by two lines will likely result in a single hunk for the user, but
in two hunks for our algorithm. On the other hand, being this strict makes the
warning consistent. We could consider using the user's diff context size in the
algorithm, but then it would depend on the current context size whether the
warning appears, which could be confusing. Plus, it would make the algorithm
quite a bit more complicated.
There are two possible fixes for this bug, and they differ in behavior when
rewording a commit. The one I chose here always splits at the first line feed,
which means that for an improperly formatted commit message such as this one:
This is a very long multi-line subject,
which you shouldn't really use in git.
And this is the body (we call it "description" in lazygit).
we split after the first line instead of after the first paragraph. This is
arguably not what the original author meant, but splitting after the first
paragraph doesn't really work well in lazygit, because we would try to put both
lines into the one-line subject field of the message panel, and you'd only see
the second and not even know that there are more.
The other potential fix would have been to join subject and description with two
line feeds instead of one in JoinCommitMessageAndDescription; this would have
fixed our bug in the same way, but would result in splitting the above message
after the second line instead of the first. I think that's worse, so I decided
for the first fix.
While we're at it, simplify the code a little bit; strings.Cut is documented to
return (s, "") when the separator is not found, so there's no need to do this on
our side.
We do have to trim spaces on the description now, to support the regular reword
case where subject and body are separated by a blank line.
SplitCommitMessageAndDescription splits at the first '\n\n' that it finds (if
there is one), which in this case is between the two paragraphs of the
description. This is wrong.
This PR captures the code coverage from our unit and integration tests. At the
moment it simply pushes the result to Codacy, a platform that assists with
improving code health. Right now the focus is just getting visibility but I want
to experiment with alerts on PRs when a PR causes a drop in code coverage.
To be clear: I'm not a dogmatist about this: I have no aspirations to get to
100% code coverage, and I don't consider lines-of-code-covered to be a perfect
metric, but it is a pretty good heuristic for how extensive your tests are.
The good news is that our coverage is actually pretty good which was a surprise
to me!
As a conflict of interest statement: I'm in Codacy's 'Pioneers' program which
provides funding and mentorship, and part of the arrangement is to use Codacy's
tooling on lazygit. This is something I'd have been happy to explore even
without being part of the program, and just like with any other static analysis
tool, we can tweak it to fit our use case and values.
## How we're capturing code coverage
This deserves its own section. Basically when you build the lazygit binary you
can specify that you want the binary to capture coverage information when it
runs. Then, if you run the binary with a GOCOVERDIR env var, it will write
coverage information to that directory before exiting.
It's a similar story with unit tests except with those you just specify the
directory inline via `-test.gocoverdir`.
We run both unit tests and integration tests separately in CI, _and_ we run them
parallel with different OS's and git versions. So I've got each step uploading
the coverage files as an artefact, and then in a separate step we combine all
the artefacts together and generate a combined coverage file, which we then
upload to codacy (but in future we can do other things with it like warn in a PR
if code coverage decreases too much).
Another caveat is that when running integration tests, not only do we want to
obtain code coverage from code executed by the test binary, we also want to
obtain code coverage from code executed by the test runner. Otherwise, for each
integration test you add, the setup code (which is run by the test runner, not
the test binary) will be considered un-covered and for a large setup step it may
appear that your PR _decreases_ coverage on net. Go doesn't easily let you
exclude directories from coverage reports so it's better to just track the
coverage from both the runner and the binary.
The binary expects a GOCOVERDIR env var but the test runner expects a
test.gocoverdir positional arg and if you pass the positional arg it will
internally overwrite GOCOVERDIR to some random temp directory and if you then
pass that to the test binary, it doesn't seem to actually write to it by the
time the test finishes. So to get around that we're using LAZYGIT_GOCOVERDIR and
then within the test runner we're mapping that to GOCOVERDIR before running the
test binary. So they both end up writing to the same directory. Coverage data
files are named to avoid conflicts, including something unique to the process,
so we don't need to worry about name collisions between the test runner and the
test binary's coverage files. We then merge the files together purely for the
sake of having fewer artefacts to upload.
## Misc
Initially I was able to have all the instances of '/tmp/code_coverage' confined
to the ci.yml which was good because it was all in one place but now it's spread
across ci.yml and scripts/run_integration_tests.sh and I don't feel great about
that but can't think of a way to make it cleaner.
I believe there's a use case for running scripts/run_integration_tests.sh
outside of CI (so that you can run tests against older git versions locally) so
I've made it that unless you pass the LAZYGIT_GOCOVERDIR env var to that script,
it skips all the code coverage stuff.
On a separate note: it seems that Go's coverage report is based on percentage of
statements executed, whereas codacy cares more about lines of code executed, so
codacy reports a higher percentage (e.g. 82%) than Go's own coverage report
(74%).
For the "cli" and "tui" modes of the test runner there's a "-race" parameter to
turn it on; for running tests on CI with go test, you turn it on by setting the
environment variable LAZYGIT_RACE_DETECTOR to a non-empty value.
Previously there was no way to render a view's search status without also moving the cursor
to the current search match. This caused issues where we wanted to display the status
after leaving the view and coming back, or when beginning a new search from within the
view.
This commit separates the two use cases so we only move the cursor when we're actually
selecting the next search match
This can be useful when you know that a cherry-picked commit would conflict at
the tip of your branch, but doesn't at the beginning of the branch (or
somewhere in the middle). In that case you want to be able to edit the commit
before where you want to insert the cherry-picked commits, and then paste to
insert them into the todo list at that point.
This prevents commands like "go test ./..." from looking into it, and it
prevents VS Code's Problems panel from showing errors about the go files in that
folder.
Add co-author to commits
Add addCoAuthor command for commits
- Implement the `addCoAuthor` command to add co-authors to commits.
- Utilize suggestions helpers to populate author names from the suggestions list.
- Added command to gui at `LocalCommitsController`.
This commit introduces the `addCoAuthor` command, which allows users to easily add co-authors to their commits. The co-author names are populated from the suggestions list, minimizing the chances of user input errors. The co-authors are added using the Co-authored-by metadata format recognized by GitHub and GitLab.
This should already have been done when adding the "View divergence from
upstream" command, but now we're going to add yet another item to the menu that
is unrelated to setting or unsetting the upstream.
This fixes two minor problems with the prompts:
1. When pressing shift-A in the local commits view, it would first prompt
whether to stage all files, and then it would prompt whether to amend the
commit at all. This doesn't make sense, it needs to be the other way round.
2. When pressing shift-A on the head commit in an interactive rebase, we would
ask whether they want to amend the last commit, like when pressing shift-A in
the files view. While this is technically correct, the fact that we're
amending the head commit in this case is just an implementation detail, and
from the user's point of view it's better to use the same prompt as we do for
any other commit.
To fix these, we remove the confirmation panel from AmendHelper.AmendHead() and
instead add it at the two call sites, so that we have more control over this.
When editing a commit, the index of the current commit can change; for example,
when merge commits are involved, or when working with stacked branches where
"update-ref" commands may be added above the selected commit.
Reselect the current commit after pressing "e"; this requires doing the refresh
blocking on the main thread. (Another option that I considered was to use a
SYNC refresh, and then select the new line with an OnUIThread inside the Then
function. This also works, but results in a very visible lag.)
This also fixes a bug where after the rebase each commit in the commits view had a tick against it because we hadn't
refreshed the view since the base commit was no longer marked
If the command used by OSCommand.OpenLink fails, lazygit crashes. With this change, if the OpenLink command fails, lazygit just shows a dialog inviting the user to visit the relevant URL.
Fixes#2882
This was on oversight on my part: I assumed that the --work-tree arg was
always intended for use with linked worktrees which have a .git file
pointing back to the repo.
I'm honestly confused now: seems like there are three kinds of worktrees:
* the main worktree of a non-bare repo
* a linked worktree (with its own gitdir in the repo's worktrees/ dir)
* a random folder which you specify as a worktree with the --work-tree arg
I'm pretty sure the --work-tree arg is only intended to be used with this
third kind or workree
I've been thinking about this for a while: I think it looks really cool if nuking your working tree
actually results in a nuke animation.
So I've added an opt-out config for it
This allows to do the equivalent of "git rebase --onto <target> <base>", by
first marking the <base> commit with the new command, and then selecting the
target branch and invoking the usual rebase command there.
It's tricky to get this right for reflog commits wrt what's the current branch
for each one; so just disable it entirely here, it's probably not something
anybody needs here.
We want to mark all local branch heads with a "*" in the local commits panel, to
make it easier to see how branches are stacked onto each other. In order to not
confuse users with "*" markers that they don't understand, do this only for the
case where users actually use stacked branches; those users are likely not going
to be confused by the display. This means we want to filter out a few branch
heads that shouldn't get the marker: the current branch, any main branch, and
any old branch that has been merged to master already.
- check out a non-main branch before we start
- add authors to expected commits so that we can see whether the commits show an
asterisk
- explicitly check what the top line displays after bisecting has started
This shows that the detached head shows an asterisk, which we don't want. We'll
fix that in the next commit.
This test not only tests the correct handling and display of the updateRef
command, but also the visualization of branch heads in the commits panel. Since
we are about to change the behavior here, extend the test so that a master
commit is added (we don't want this to be visualized as a branch head), and then
a stack of two non-main branches. At the end of this branch we only want to
visualize the head commit of the first.
From the go 1.19 release notes:
Command and LookPath no longer allow results from a PATH search to be found relative to the current directory. This removes a common source of security problems but may also break existing programs that depend on using, say, exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe) in the current directory. See the os/exec package documentation for information about how best to update such programs.
We've been sometimes using lo and sometimes using my slices package, and we need to pick one
for consistency. Lo is more extensive and better maintained so we're going with that.
My slices package was a superset of go's own slices package so in some places I've just used
the official one (the methods were just wrappers anyway).
I've also moved the remaining methods into the utils package.
In the presentation layer, when showing branches, we'll show worktrees against branches if they're
associated. But there was a race condition: if the worktree model was refreshed after the branches model,
it wouldn't be used in the presentation layer when it came time to render the branches.
A better solution would be to have some way of signalling that a particular context needs to be refreshed
and after all the models are done being refreshed, we then refresh the contexts. This will prevent
double-renders
When switching worktrees (which we can now do via the branch view) we re-layout the windows and their views.
We had the worktree view ahead of the file view based on the Flatten() method in context.go, because it used
to be associated with the branches panel.
There are quite a few paths you might want to get e.g. the repo's path, the worktree's path,
the repo's git dir path, the worktree's git dir path. I want these all obtained once and
then used when needed rather than having to have IO whenever we need them. This is not so
much about reducing time spent on IO as it is about not having to care about errors every time
we want a path.
We now always re-use the state of the repo if we're returning to it, and we always reset the windows to their default tabs.
We reset to default tabs because it's easy to implement. If people want to:
* have tab states be retained when switching
* have tab states specific to the current repo retained when switching back
Then we'll need to revisit this
For marking as good or bad, the current commit is pretty much always the one you
want to mark, not the selected. It's different for skipping; sometimes you know
already that a certain commit doesn't compile, for example, so you might
navigate there and mark it as skipped. So in the case that the current commit is
not the selected one, we now offer two separate menu entries for skipping, one
for the current commit and one for the selected.
This can be useful if you want to find the commit that fixed a bug (you'd use
"broken/fixed" instead of "good/bad" in this case), or if you want to find the
commit that brought a big performance improvement (use "slow/fast"). It's pretty
mind-bending to have to use "good/bad" in these cases, and swap their meanings
in your head.
Thankfully, lazygit already had support for using custom terms during the bisect
(for the case that a bisect was started on the command-line, I suppose), so all
that's needed is adding a way to specify them in lazygit.
Previously we used a single-line prompt for a tag annotation. Now we're using the commit message
prompt.
I've had to update other uses of that prompt to allow the summary and description labels to
be passed in
Previously, we would only show the authors based on local commits, but sometimes you want to set a commit author
to that of a commit on another branch. Now, so long as you've viewed the branch's commits, the author will appear
as a suggestion.
We have a use-case to rebind 'm' to the merge action in the branches panel. There's three ways to handle this:
1) For all global keybindings, define a per-panel key that invokes it
2) Give a name to all controller actions and allow them to be invoked in custom commands
3) Allow checking for merge conflicts after running a custom command so that users can add their own 'git merge' custom command
that matches the in-built action
Option 1 is hairy, Option 2 though good for users introduces new backwards compatibility issues that I don't want to do
right now, and option 3 is trivially easy to implement so that's what I'm doing.
I've put this under an 'after' key so that we can add more things later. I'm imagining other things like being able to
move the cursor to a newly added item etc.
I considered always running this hook by default but I'd rather not: it's matching on the output text and I'd rather something
like that be explicitly opted-into to avoid cases where we erroneously believe that there are conflicts.
It seems that older git versions would drop empty commits when rebasing. Since
this aspect is not relevant to what we're testing here, fix this by simply
avoiding empty commits in these tests.
The test apply_in_reverse_with_conflict.go fails in git versions 2.30.8 and
earlier. Apparently the output "Applied patch to 'file2' cleanly" was only added
more recently. It's not essential that we check this output.
Older versions of git don't support the -b option yet. However, no version of
git complains about the -c option, even when the init.defaultBranch config is
not supported.
For older git versions we won't be able to support any other main branch than
"master", so hard-code that in Init.
This doesn't fix anything for older versions yet; see the next commit for that.
Now that we are running each test 6 times on CI, the risk of flakiness
is higher. I want to fix these tests for good but it'l take time, so
we're just retrying for now
I don't know why we're getting index.lock errors but they're impossile to stop
anyway given that other processes can be calling git commands. So we're retrying
a few times before re-raising. To do this we need to clone the command and the current
implementation for that is best-effort.
I do worry about the maintainability of that but we'll see how it goes.
Also, I thought you'd need to clone the task (if it exists) but now I think not;
as long as you don't call done twice on it you should be fine, and you shouldn't
be done'ing a task as part of running a command: that should happen higher up.
I want to see how we go removing all retry logic within a test. Lazygit should be trusted to tell us when it's no longer busy,
and if it that proves false we should fix the issue in the code rather than being lenient in the tests
The remote branches controller was using its own escape method meaning it didn't go through the flow of cancelling
an active filter. It's now using the same approach as the sub-commits and commit-files contexts: defining a parent
context to return to upon hittin escape.
The title was saying "Unstage lines", which was just wrong. The text said
"Delete lines", which can be seen as a bit misleading; we are only discarding
the changes to the selected lines, not deleting the lines themselves.
For consistency, rename the config variable skipUnstageLineWarning accordingly.
This test is almost identical to swap_in_rebase_with_conflict.go, except that it
sets the commit that will conflict to "edit".
This test is interesting because there's special code needed to determine
whether an "edit" command conflicted or not, i.e. whether to show the "confl"
entry. In this case we do. We have lots of other tests already that have "edit"
commands that don't conflict, so that's covered already.
When stopping in a rebase because of a conflict, it is nice to see the commit
that git is trying to apply. Create a fake todo entry labelled "conflict" for
this, and show the "<-- YOU ARE HERE ---" string for that one (in red) instead
of for the real current head.
This test is interesting because it already behaves as desired: since git has
rescheduled the "pick" command, we do _not_ want to show a "conflict" entry in
this case, as we would see the same commit twice then.
We use CommitFilesController also for the files of commits that we show
elsewhere, e.g. for branch commits, tags, or stashes. It doesn't make sense to
discard changes from those (for stashes it might be possible to implement it
somehow, but that would be a new feature), so we disallow it unless we are in
the local commits panel.
I don't know why we were setting the initial context to CurrentSideContext
and not just CurrentContext in the first place. If there is no current context
in either case it'll default to the files context. So the only issue is if
we anticipated that some random context would be focused and we didn't want to
activate that. But I can't think of any situation where that would happen.
Whenever we perform an action in a test, we should assert on the result before doing the next action.
This prevents issues where the test moves too fast for our code. It would be nice to not have to do this,
but for now that's the situation
