cli: Move external git repo canonicalization into Workspace::init_g… #2830
Conversation
…it_external * Move canonicalization of the external git repo path into the Workspace::init_git_external(). This keeps necessary code together. * Add a new variant of WorkspaceInitError for reporting path not found errors. The user error string is written to pass existing tests.
essiene
force-pushed
the
essiene/2747/2
branch
from
3a6db44 to
938bf01
Compare
essiene
changed the title
| git_repo_path.pop(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm not strongly against this change, but I think ".git" should be appended by CLI code. git_repo_path here should point to the git repository, not the workspace.
There was a problem hiding this comment.
I think you're right. I saw the canonicalization below and figured it was consistent to normalize the git_repo_path here too, but I missed the TODO saying that we planned to move the canonicalization below out of this function.
There was a problem hiding this comment.
Circling back to clean things up.
I think I've managed to get myself confused by the terms. Help check my understanding:
Workspace path - This is the directory the user works in. It contains the 'jj' repo and may also contain a 'git' repo.
Repo path - This holds the "special" folder for git/jj, so for jj, this is the path to the .jj directory, but for git, it's the path to the .git directory.
Is that correct?
What should git_repo_path point to then?
I guess, technically, we want it to point to the '.git' directory, but for the user, it's a bit annoying UI wise, so we can accept the parent directory and normalize it ourselves.
Am I understanding this correctly?
Thanks! :-D
There was a problem hiding this comment.
Workspace path - This is the directory the user works in. It contains the 'jj' repo and may also contain a 'git' repo.
Repo path - This holds the "special" folder for git/jj, so for jj, this is the path to the .jj directory, but for git, it's the path to the .git directory.
Basically yes, that's my understanding. For jj, the repo path is probably .jj/repo. For "bare" git repo, the path might not end with ".git".
What should
git_repo_pathpoint to then?I guess, technically, we want it to point to the '.git' directory, but for the user, it's a bit annoying UI wise, so we can accept the parent directory and normalize it ourselves.
Yes. That's why I insist that it's CLI/UI business to append ".git".
FWIW, we call -R/--repostiroy a repository path in CLI, but it's actually a workspace path.
Move the user input canonicalization closer to where it is actually used, so the code reads better as a whole.
Split out from #2807