Skip to content

Conversation

@jesseduffield
Copy link
Owner

@jesseduffield jesseduffield commented Feb 25, 2023

  • PR Description
  • adds some better constructs for testing the staging panel (e.g. asserting on a range of selected lines)
  • updates the patch explorer code to remove an extra line at the bottom that shouldn't have been there
  • fixes race condition in gocui code

I'm gonna have a separate PR which updates some method names to make things more consistent.

  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • Docs (specifically docs/Config.md) have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

continue
}

if lineIdx == len(lines)-1 && line == "" { // skip the trailing newline
Copy link
Owner Author

Choose a reason for hiding this comment

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

I noticed that you can actually select past the bottom of the patch thank to a trailing newline that we treat as a separate line in its own right. I've got a couple of changes in this PR that trim that trailing newline, and from all my testing it seems like a safe change. We still ensure that upon copying the patch to the clipboard it's got a trailing newline in it.

name: fmt.Sprintf("contains '%s'", target),
testFn: func(value string) (bool, string) {
// everything contains the empty string so we unconditionally return true here
if target == "" {
Copy link
Owner Author

Choose a reason for hiding this comment

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

no idea why this code was needed: I thought strings.Contains would do the job properly but apparently not

func (self *TestDriver) navigateToListItem(matcher *matcher) {
self.inListContext()

currentContext := self.gui.CurrentContext().(types.IListContext)
Copy link
Owner Author

Choose a reason for hiding this comment

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

turns out it doesn't matter if we're in a list item or not. I'll update this function to be called navigateToLine in a separate PR

@github-actions
Copy link
Contributor

github-actions bot commented Feb 25, 2023

Uffizzi Preview Environment deployment-17400

☁️ https://app.uffizzi.com/github.com/jesseduffield/lazygit/pull/2474

📄 View Application Logs etc.

What is Uffizzi? Learn more

return self
}

func (self *ViewDriver) ContainsLines(matchers ...*matcher) *ViewDriver {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Now we have Lines, TopLines, ContainsLines, and SelectedLines. We'll need to update the naming so they're all consistent e.g. AllLines, TopLines, SomeLines, and SelectedLines. Something like that. I'll do that in another PR

}

func (self *Views) byName(viewName string) *ViewDriver {
func (self *Views) regularView(viewName string) *ViewDriver {
Copy link
Owner Author

Choose a reason for hiding this comment

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

not really happy with the name here but needed to distinguish from a view driver that had its own special getSelectedLines function


// expected to only be used in tests
func (v *View) SelectedLine() string {
v.writeMutex.Lock()
Copy link
Owner Author

Choose a reason for hiding this comment

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

had some race conditions happening here

@jesseduffield jesseduffield merged commit 8a69d99 into master Feb 25, 2023
@delete-merged-branch delete-merged-branch bot deleted the improve-staging-tests branch February 25, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants