Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update git_root on refresh #944

Closed
wants to merge 3 commits into from

Conversation

smatting
Copy link
Contributor

@smatting smatting commented Nov 16, 2023

Fixes #931 and fixes #442

This PR updates in_prepared_repo to a different scenario (called "Scenario 2"), namely that the user changes directory to a git repo after setup() has already been called. This makes 14 tests fail, also including the tests for hunk operations which explains the issue #931.

The second commit fixes all failing tests, by making sure repository.git_root and repository.git_path is updated on every refresh of the status. Also this commit removes the unused repository.cwd.

Maybe this PR could be extended so that in_prepared_repo would run each test for both scenarios:

  1. Scenario 1: User runs setup() while already being in the right git repo (i.e. you start nvim inside the git repo).
  2. Scenario 2: User changes directory after setup() has been called (i.e. you start nvim and then :cd into the git repo)

I was not able to fit both scenarios into in_prepared_repo because running Scenario 1 first would influence the outcome of running Scenario 2, so some critical cleanup was missing between running both scenarios. Maybe somebody can help me with this? With the bugfix included in this PR all tests pass when running them in either Scenario 1 and Scenario 2 separately. We could of course also duplicate all tests, but that would be ugly. What we really want is that that every test passes in either scenario, because that assert that neogit behaves the same.

@smatting
Copy link
Contributor Author

smatting commented Nov 17, 2023

I'm no longer sure this PR makes sense, now that I have read more of the codebase: it appears like everything related to cwds is much more complicated than it has to be. Wouldn't it be much simpler if all git commands would be executed with cwd = git.repo.git_root and this state would only be updated when you call neogit.open() and only then?

@CKolkey
Copy link
Member

CKolkey commented Nov 18, 2023

Hmm. Yeah, sounds reasonable to me. A lot of the CWD logic is from before I got involved with this project. If you think there's a better (simpler) way, I'd love to see what you have in mind :)

@smatting smatting closed this Nov 19, 2023
@smatting
Copy link
Contributor Author

@CKolkey I manged to pull it off: #963

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.

Changing CWD breaks action on hunks Can't stage a hunk from file up a dir from PWD
2 participants