The long story: I want to call this function from RefsHelper; however, I can't
make WorkingTreeHelper a field of RefsHelper because RefsHelper is already a
field in WorkingTreeHelper, so that would be a circular dependency.
The shorter story: there's really little reason to have to instantiate a helper
object in order to call a simple function like this. Long term I would like to
get to a state where a lot more of these helper functions are free-standing, and
you pass in the data they need.
While at it, simplify the implementation of AnyStagedFiles and AnyTrackedFiles
to one-liners.
It's the same, really, except that GetCheckedOutRef() does a check if any
branches exist and returns nil if not. Since we are accessing the returned
branch unconditionally without checking for nil, it seems this check is not
needed here. (The functions we are touching here are called from handlers that
are guarded with itemSelected or singleItemSelected, so we know that at least
one branch exists.)
The goal is to get rid of the dependency to refsHelper.
When debugging an integration test that involves some behind-the-scenes
rebasing, the daemon lazygit would also wait for a debugger to attach. This is
very confusing because the test seems to hang; once you figured out what's going
on, it's inconvenient because you need to attach a debugger to the daemon every
time you debug the test.
Now, it would sometimes be useful to be able to debug the daemon itself (whether
inside an integration test, or during normal usage), and I have often wished to
be able to do that. We might introduce an additional env var (and command-line
option?) to enable this; but that's out of scope here.
- **PR Description**
The integration test `stash/rename.go` occasionally fails for me
locally. This PR fixes it.
In 8b8343b8a9f we made a change to run newPtyTask from AfterLayout; this
is needed so that the PTY gets the new, updated view size. However, this
created a race condition for integration tests that select a line in a
list view and then expect the main view to have certain content;
sometimes that content gets rendered too late.
I'm surprised that this didn't cause more tests to fail; right now I
only know of one test that occasionally fails because of this, which is
`stash/rename.go`.
Fix this by moving the AfterLayout to inside newPtyTask, and do it only
when we are actually using a PTY (we don't when no pager is configured,
which is the case for integration tests).
The diff is best viewed with "ignore whitespace" turned on.
In 8b8343b8a9f we made a change to run newPtyTask from AfterLayout; this is
needed so that the PTY gets the new, updated view size. However, this created a
race condition for integration tests that select a line in a list view and then
expect the main view to have certain content; sometimes that content gets
rendered too late.
I'm surprised that this didn't cause more tests to fail; right now I only know
of one test that occasionally fails because of this, which is stash/rename.go.
Fix this by moving the AfterLayout to inside newPtyTask, and do it only when we
are actually using a PTY (we don't when no pager is configured, which is the
case for integration tests).
The diff is best viewed with "ignore whitespace" turned on.
- **PR Description**
* The keybind for moving a commit up in the interactive rebase example
of the README file is incorrect.
I've changed it to the keybind present in current versions of Lazygit
- **PR Description**
When selecting a range of commits by selecting the top one and then
pressing shift-down to create a range, and then pressing enter and
checking out one of the files by pressing `c`, you would get the file
checked out with its state at the bottom end of the range, which is not
what you want; it is expected to check out the file at the state that
the diff shows it changes to. (And if the file was only created in the
middle of that range, trying to check it out would result in an error.)
Fix this by paying attention to a "from-to" range selection, and
checking out the file at the "to" state.
Related to #4420.
The test shows a misbehavior: even though the diff shows "-one" and "+three",
meaning that "three" is the state we want to check out, we get "one". The reason
is that the checkout file command doesn't pay attention to range selections, it
only looks at the "moving end" of the range. Had we created the range by
selecting "two" and then pressed shift-up to "three", we would have gotten the
expected result.
- **PR Description**
When using gpg signing on tags via the git config `tag.gpgSign=true`, it
is not possible to do a lightweight tag. You must do an annotated tag
(Which is allowed to have an empty description).
This PR augments the existing `WithGpgHandling` helper to allow it to
specifically target commit signing and tag signing. It then uses that
handler, and creates an annotated tag, when the git config demands tag
GPG signing. By default it will launch the tag signing in a new
subprocess. This does require that users click an extra `<enter>` after
creating a signed tag, but this is currently the behavior for signed
commits, so I don't feel too bad about that. If they want, they can use
the LazyGit configuration option of `git.overrideGpg: true` to promise
to LazyGit that they do not need a GPG sub-process (because they
reliably already have gpg-agent or similar running in the background).
This again matches the current behavior for GPG signed commits.
This has no integration test because we don't have the machinery set up
in place to set up a GPG key inside of our integration test framework.
Fixes https://github.com/jesseduffield/lazygit/issues/2955
The actual usage is case insensitive, so this doesn't actually matter.
But if fills my heart with joy.
The test is case sensitive, but the actual response to `git config
commit.gpgSign` is equivalent to `git config commit.gppsign`
- **PR Description**
This removes generated Config.md entries that are contain the word:
`deprecated` (case insensitive) anywhere in their header comment.
I think adding in deprecated configs into the reference config clutters
it at best, and encourages users to use those configs at worst. They are
still going to be validated by the schema, but this PR removes them from
the generated config.
I also added in the missing `deprecated` yaml tags in our user config in
anticipation of https://github.com/invopop/jsonschema/pull/79 getting
merged.
- **PR Description**
Include empty arrays and maps in the generated Config.md
It's not an ideal solution because there's no indication of what kind of objects
you can add to those maps or arrays, but at least they show up at all (with a
comment containing a link to more information), and that's already an improvement.
It's not an ideal solution because there's no indication of what kind of objects
you can add to those maps or arrays, but at least they show up at all (with a
comment containing a link to more information), and that's already an
improvement.
As discussed in #1984, this PR modifies the code such that the `w`
command always commits without pre-commit hooks. The `skipHookPrefix` is
still considered when starting the commit messaged with the prefix in
the regular commit command using `c`.
Skipping pre-commits also works when switching to the editor and adding
a co author from the the `w` command
Add verify flag
Add and update integration tests
Rename verify to forceSkipHooks
Adapt CommitSkipHooks integration test to actually use a hook
Remove forceSkipHooks param from OnConfirm et al
Simplify tests
- **PR Description**
This PR adds a new bulk action for submodules.
As I often work with a lot of submodules that are hierarchically
structured, the flag "--recursive" saves a lot of work.
- **PR Description**
Since they started releasing .0 versions with 1.21.0, the go version
need to be a full release version specifier and not a go language
version. They relented and added support for defaulting to a .0 release
if none is specified in later 1.23.x releases, but for users with local
1.21 or 1.22 toolchains the lack of a full release specifier breaks
them. And going forwards this is more technically correct anyway.
See https://github.com/golang/go/issues/62278#issuecomment-1693620853
Since they started releasing .0 versions with 1.21.0, the go version
need to be a full release version specifier and not a go language
version. They relented and added support for defaulting to a .0 release
if none is specified in later 1.23.x releases, but for users with local
1.21 or 1.22 toolchains the lack of a full release specifier breaks
them. And going forwards this is more technically correct anyway.
See https://github.com/golang/go/issues/62278#issuecomment-1693620853
Assert the entire lines using Equals instead of Contains. This makes the tests a
bit easier to read, and it makes it much easier to decide how they need to be
changed when we change the layout (like we do in the last commit of this
branch).
It is true that this requires changing all these tests for any future UI
changes, but I think this is a good price to pay; those adaptions are trivial
and can be done without thinking.
