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

Remove dependency on cwd #963

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Conversation

smatting
Copy link
Contributor

@smatting smatting commented Nov 22, 2023

This pull request fixes #931 and fixes #442 by ensuring the independence of nearly all library functions from the current working directory (cwd).

Specifically, this pertains to functions that directly or indirectly use cwd, which can change anytime, either manually by the user or automatically through plugins. Cwd dependent functions make it difficult to reason about the state of Neogit and potentially lead to numerous bugs. Throughout the codebase, you can observe various attempts to circumvent this issue, often involving the tracking of the cwd via state.cwd.

Notably, all functions in neogit.lib.git.cli are dependent on the cwd, with require("neogit.lib.git.cli").git_root() being a noteworthy example.

This pull request aims to eliminate all cwd dependencies by designating require("neogit.lib.git.repository").git_root as the source of truth for the git root that Neogit is targeting. Unlike the cwd, which can change at any point, repository.git_root only updates through repository.refresh() triggered by manual user intervention (e.g., Ctrl+R or :Neogit) and should - hopefully - also invalidate all other state which depends on the git root.

Key changes in this pull request include:

  • Ensuring that all cli functions are executed with cwd = repository.git_root. This change is particularly impactful as it ensures that all cwd-relative file paths returned by git do not need to be converted to paths relative to the git root, as cwd == git_root always holds.
  • Replacing all instances of cli.git_root() with repository.git_root in the codebase to maintain consistency.
  • Renaming cli.git_root() to cli.git_root_of_cwd() to discourage its use and prevent undoing the modifications introduced by this pull request.
  • Removing workaround code related to changing cwd, such as diffview.root_prefix(), repository.cwd, and the use_git_root argument of hunk operations.

Smaller changes include:

  • Renaming git_is_repository_sync to is_inside_worktree for clarity.
  • Removing neogit.get_repo() due to its solitary use.
  • Eliminating unused functions cli.git_root_sync() and cli.git_dir_path_sync().
  • Making repository.git_path a function of the module rather than part of the state, as git_path depends on git_root.

Additionally, two tests that relied on Neogit being cwd dependent were adjusted to ensure that status.reset() is called after the cwd has been changed.

@smatting smatting changed the title Remove dependencies on cwd Remove dependency on cwd Nov 22, 2023
@CKolkey
Copy link
Member

CKolkey commented Nov 22, 2023

Very insightful and thorough. I do love a PR that removes nearly 2x the lines it adds.

Thank you for the contribution :)

@CKolkey CKolkey merged commit 91a2655 into NeogitOrg:master Nov 22, 2023
3 checks passed
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