These are very similar in that they call RunAndProcessLines on a git log command
and then process lines to create commits. Extract this into a common helper
function. At the moment this doesn't gain us much, but in the next commit we
will extend this helper function considerably with filter path logic, which
would otherwise have to be duplicated in both places.
It is good practice to use a -- argument even if no further arguments follow.
Doesn't really make a difference for this particular command though, I think.
I don't know why this function argument was added, but I don't like unnecessary
indirections, so I'm removing it as SubCommitsHelper has access to everything it
needs to do it itself.
Recycle reflog commits only for the non-filtered ones.
If we're not filtering, FilteredReflogCommits and ReflogCommits are the same. If
we then enter filtering and get reflog entries again, and pass a
lastReflogCommit, we'd recycle the previous FilteredReflogCommits, which are
unfiltered, so that's no good. Work around this by simply never using the
recycle mechanism when getting the filtered reflog commits.
We could do better by remembering what the last filter path or author was, and
only suppressing the recycling when it changed; but that's more complexity than
I want to add, so hopefully this is good enough.
Using the filtered one is probably not a good idea. It didn't do much harm
because the split of ReflogCommits and FilteredReflogCommits doesn't really work
right now (FilteredReflogCommits is always the same as ReflogCommits, even in
filtering mode), but we'll fix this in the next commit.
I can only guess what happened here: in aa750c0819, this code to manually load
the reflog commits was added, to make sorting branches by recency work when the
reflog is filtered by path. At that time we didn't have separate ReflogCommits
and FilteredReflogCommits models yet. Then, FilteredReflogCommits was introduced
(in 8822c409e2), probably for the very purpose of being able to get rid of this
again; but then it was forgotton to actually get rid of it.
The funny thing is that the introduction of FilteredReflogCommits happened in
the very next commit, 15 minutes later.
The test shows that selecting a commit before the rename shows an empty diff,
and selecting the rename itself shows an added file rather than a rename.
This broke with the introduction of the age in the stashes list in bc330b8ff3
(which was included in 0.41, so 12 minor versions ago).
This makes it work again when filtering by file, but it still doesn't work when
filtering by folder (and never has). We'll fix that next.
This was not a good idea, and it seems it has never been tested: the --name-only
and -z options don't work well together. Specifically, -z seems to simply cancel
--name-only.
This is not enough to make it work, we'll need more fixes in the next commit.
This reverts commit 50044dd5e0.
The test shows that it doesn't work, the list is empty both when filtering by
file and by folder.
I could have added test cases for these to the unit tests in
stash_loader_test.go instead, but I don't find tests that need to mock git's
output very valuable, so I opted for an integration test even though it takes
much longer to run.
Since filtering switches to half-screen mode in the local commits panel, most
people probably didn't notice, but we do also filter those other views. So when
leaving half-screen mode (but not filtering), you could switch to sub-commits or
stashes, and those would show the filtered view only after the next refresh
(e.g. after a background fetch). It's worse when leaving filtering, because this
goes back to normal screen mode, and you would often see an empty stashes panel
after that (until the next background fetch), which is quite confusing.
I also find it questionable to always switch focus to the commits panel when
entering filtering. If it is initiated from subcommits, reflog, or stashes,
maybe we want to stay there. I'm not changing this now since I'm unsure how much
people rely on the current behavior.
When exiting filtering mode while the focus is not in the local commits panel,
the call to HandleFocus would render the selection in the commits panel as if
the panel had the focus.
The call to HandleFocus was introduced recently in 4981419ba9 in an attempt to
rerender the main view correctly, but it should only have been called when local
commits is the focused context. But instead, we can use PostRefreshUpdate, which
handles this distinction.
- **PR Description**
Instead of requiring the user to install the right version of the tool
in their .bin folder, create a shim that automatically runs the right
version of the tool. This has several benefits:
- it works out of the box with no setup required (the tool will be
automatically downloaded and compiled the first time it is used)
- no work needed for developers when we bump the golangci-lint version
- it works in working copies that are used in different environments
(e.g. locally on a Mac, or inside a dev container)
Approach suggested by @kyu08.
Instead of requiring the user to install the right version of the tool in their
.bin folder, create a shim that automatically runs the right version of the
tool. This has several benefits:
- it works out of the box with no setup required (the tool will be automatically
downloaded and compiled the first time it is used)
- no work needed for developers when we bump the golangci-lint version
- it works in working copies that are used in different environments (e.g.
locally on a Mac, or inside a dev container)
Co-authored-by: kyu08 <49891479+kyu08@users.noreply.github.com>
- **PR Description**
Add support for Alt+Backspace (Option+Backspace on macOS) to delete
entire words in text input areas, matching common text editor behavior.
- **PR Description**
This is a small PR that extends the logic added in
https://github.com/jesseduffield/lazygit/pull/4704
There were many reports of users accidentally resetting the working tree
using the reset commands and a nice change was added by @stefanhaller to
prompt for confirmation on those actions but the "Nuke working tree"
option was not covered by the PR.
I believe this one is actually one of the most important options to
prompt because it's by default the first one on the list. I myself
triggered it once when trying to discard a single file while unknowingly
having caps-lock enabled as I thought I was confirming the single file
discard option.
## **PR Description**
I added a new feature to checkout to the last branch.
I think it would be convenient to be able to checkout to the last branch
with just one key stroke(`-`) in some cases, such as when switching
between multiple branches during code review.
Off course, I know that you can same thing by entering `"-"` to the text
input of "Checkout by name" or using custom command but I think `git
checkout -` is occasional use case. So I feel this feature can be a part
of default features.
Another advantage of this feature is that you can checkout to the last
branch regardless of cursor position.
Being able to switch branches without worrying about cursor position
reduces stress I think.
We forgot to convert the model indices to view indices in searchModelCommits.
This needs to be done for search results to be highlighted correctly in the
"divergence from upstream" view, which adds "--- Remote/Local ---" entries, and
during a rebase, where we have "--- Pending rebase todos ---" and "--- Commits
---" which offset view indices from model indices.
I have seen cases where during a rebase (two nonModelItems) all entries in
viewIndicesByModelIndex beyond the second nonModelItem were off by 4 rather than
2 as I would expect. The only explanation I have for this is that the function
was called concurrently.
Improve this by working on a local variable and only assign to self at the end;
this is not a real fix for the concurrency issue of course, but it makes it much
less likely to be a problem in practice.
- **PR Description**
In prompts with a suggestions panel (e.g. the "Execute shell command"
window, or the "Filter by path" panel), you can now
- scroll the list of suggestions with the mouse wheel even when the
focus is in the edit field
- click in the suggestions list or in the edit field to switch focus
between them
- double-click a suggestion to trigger it
This doesn't change behavior, just makes the code a little bit easier to
understand: the outermost condition is "do we show a popup and is the mouse
event for some other view than the focused one". Only if that's true do we need
to define the isCommitMessageView function, and check whether both views belong
to the commit message editor.
- **PR Description**
Previously, any click on an already selected line in a list view was
treated as a double click, and triggered the same action as pressing
either enter or space, depending on the view. Improve this by doing
proper double-click detection based on how much time has passed between
clicks.
Fixes#2526.
Previously a click was detected as a double-click whenever the click was on the
already selected line, regardless of how long ago the last click was (or even
when it wasn't selected by clicking at all). Now that gocui supports proper
double-click detection, we can do better.
The rationale for this is the same as in the previous commit; however, for these
functions we only allow a single controller to set them, because they are event
handlers and it doesn't make sense for multiple controllers to handle them.
Trying to do this would previously have the second one silently overwrite the
first one's.
We don't currently have this in lazygit, but I ran into the situation once
during development, and it can lead to bugs that are hard to diagnose.
Instead of holding a list of functions, we could also have added a panic in case
the function was set already; this would have been good enough for the current
state, and enough to catch mistakes early in the future. However, I decided to
allow multiple controllers to attach these functions, because I can't see a
reason not to.
These are never called on the context, they only exist to satisfy the
HasKeybindings interface. But that's wrong, HasKeybindings has nothing to do
with focus handling or rendering the main view. Remove them from that interface
and add them to IController instead.
Similar to what was done in 457cdce61d, and for the same reason.
However, instead of waiting and fixing them one by one as we see them fail, I
decided to go about it more systematically. To do that, I added calls to
`time.Sleep(1 * time.Second)` in all the Shell.Commit* helper functions; this
ensures that all the commits we make get different committer time stamps, making
all these tests fail. With this I'm pretty confident that we're good now.
When entering filtering we would only call FocusLine, which takes care of
highlighting the selected line in the commits list, but not of re-rendering the
main view. HandleFocus does that.
When exiting filtering, the HandleFocus call was missing entirely.
The tests needed to be reworked a little bit to make this testable.
Now that -committerdate is the default sort order, we could get different
results for the sort order of the branches list depending on whether the commits
on both branches have the same committer time stamp (likely in an integration
test, since git time stamps have second resolution), in which case git will fall
back to alphabetical order, or not (rare, but possible), in which case master
will have the newer commit and will come first. Make this stable by forcing the
sort order to alphabetical.
We might have more tests with this problem, we'll just have to fix them one by
one as we see them fail.
As discussed in #4602, we make a general change to how we treat UI
settings that can be changed from within lazygit; previously, the rule
was to persist them in State.yml so that they automatically remember the
last value that was set in the UI; now, we no longer do this, and
instead provide user configs for setting defaults which we fall back to
on every start.
See #4602 for an in-depth discussion about the pros and cons of either
approach, and why we chose this one.
As part of this, we change the default for the sort order of branches to
date (aka committerdate), both for local and for remote branches (was
recency and alphabetical before, respectively). See the discussion
starting
[here](https://github.com/jesseduffield/lazygit/pull/2147#issuecomment-1656700988)
for some reasons.
Fixes#4602.
At the same time, we change the defaults for both of them to "date" (they were
"recency" and "alphabetical", respectively, before). This is the reason we need
to touch so many integration tests. For some of them I decided to adapt the test
assertions to the changed sort order; for others, I added a SetupConfig step to
set the order back to "recency" so that I don't have to change what the test
does (e.g. how many SelectNextItem() calls are needed to get to a certain
branch).
When toggling the value in the UI we simply overwrite the value in UserConfig;
this would be bad if there was ever a chance that we want to write the user
config back to disk, but it is very unlikely that we can do that, because
currently we have no way to tell which parts of the config come from the global
config file and which ones come from a repo-local one.
The version choice is a little arbitrary, but see discussion at
https://github.com/jesseduffield/lazygit/pull/4559#issuecomment-2876201680.
The main reason why I'm updating the version now is that versions before 2.27
had a bug with branch sorting, where sorting by -committerdate (which will be
our default soon) would sort branches that point at the same commit in reverse
alphabetical order rather than alphabetical order. While this is only slightly
annoying but not a huge deal for users, it makes maintaining our integration
tests across versions very hard. So I wanted to update to at least 2.27 to get
around this problem, and went with 2.32 after the discussion linked to above.
The choice of which versions to run integration tests on is pretty arbitrary
too, I just picked some at random which are about 5 to 6 minor versions apart.
- **PR Description**
Doing a hard reset is really destructive because it loses all
uncommitted changes, so add a confirmation in case there are any.
Fixes#3085Fixes#3709Fixes#4688
It's a very common pattern in the code base to have some code that we want to
run either directly, or with a confirmation, depending on some condition. In
most cases this is solved by creating a local helper function that we call
either directly or from within the HandleConfirm of the confirmation; provide a
convenience helper that makes this easier.
These were found by commenting out the initialization of the struct literal in
EnglishTranslationSet.TranslationSet(), and then running
$ punused pkg/i18n/**/*.go
Punused can be installed with `go install github.com/bep/punused@latest`.
The code was copied from StagingController in 0496e3af50, and I did add the new
text in that commit, I just forgot to adapt the code to actually use it.
- **PR Description**
If the hunk to be selected was partially scrolled offscreen, the view wouldn't
scroll enough to make it completely visible (the last line of the hunk was still
offscreen).
If the hunk to be selected was partially scrolled offscreen, the view wouldn't
scroll enough to make it completely visible (the last line of the hunk was still
offscreen).
This is only a minimal fix for a pressing problem. The code to handle scrolling
after selection changes has lots of problems, and is also inconsistent between
list views and the patch explorer, but cleaning this up needs more time than I
have right now.
Globally ignoring all dot files and then making exceptions as needed to pull
files back in again is very error prone. It's better to explicitly exclude
everything we want to hide.
The reason why this came up is that I tried to look at the git history of one of
the files in .github/workflows/ using lazygit's path filtering feature, but it
didn't show up in the list of suggestions. It took me a while to realize that
that's because this list doesn't show git-ignored files. Now, .github/workflows/
wasn't really git-ignored because it was brought back by an exclamation mark
entry in the Exceptions section; but maybe the library we are using to get the
files doesn't handle these properly or something (I didn't further research
this).
Globally ignoring all dot files and then making exceptions as needed to pull
files back in again is very error prone. It's better to explicitly exclude
everything we want to hide.
This can be seen in the vendor directory, where we omitted a lot of files
accidentally (we'll fix that in the next commit). None of these were important,
so no harm done, but still.
The reason why this came up is that I tried to look at the git history of one of
the files in .github/workflows/ using lazygit's path filtering feature, but it
didn't show up in the list of suggestions. It took me a while to realize that
that's because this list doesn't show git-ignored files. Now, .github/workflows/
wasn't really git-ignored because it was brought back by an exclamation mark
entry in the Exceptions section; but maybe the library we are using to get the
files doesn't handle these properly or something (I didn't further research
this).
The core homebrew formular is usually up to date very quickly, so
there's little reason to use the custom tap.
We still maintain the tap and update it for the benefit of users who
already subscribed to it, but we no longer recommend it for new users.
The core homebrew formular is usually up to date very quickly, so there's little
reason to use the custom tap.
We still maintain the tap and update it for the benefit of users who already
subscribed to it, but we no longer recommend it.
- **PR Description**
Make the action run only in my fork, so that releases appear to be
created by me.
Also, update goreleaser to the latest version, and some other tweaks.
As far as I can tell, this is the only way to make sure that releases show up as
created by me. Also, we totally don't want it to run in other people's forks
(although it would likely just have failed there, but still).
The restriction only applies to scheduled runs; manually triggering the action
is still possible from everywhere. There needs to be a personal access token
named LAZYGIT_RELEASE_PAT configured on the repo for this to work, though.
- **PR Description**
This PR fixes the broken markdown table formatting in the auto-generated
keybindings documentation files by properly escaping newlines in
tooltips.
Issue: https://github.com/jesseduffield/lazygit/issues/4689
- **PR Description**
As of #4684, hunk mode has become so useful that I prefer it over line
mode now. This PR adds a config that lets you use hunk mode by default
in the staging view.
I'm not enabling this by default yet, although I do think it's the more
useful mode for most people. The biggest issue that I still have with
this is that _if_ you need to switch to line mode for some reason, then
it's very non-obvious how to do that. New users might not find out at
all, and think that lazygit doesn't allow staging individual lines.
Stefan Haller
