Skip to content

Fix revparsing with windows paths in blame example #722

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

enizor
Copy link

@enizor enizor commented Jun 16, 2021

revparse_single uses a git object name and not a file path - that means backslashes must be translated to slashes on windows.
Otherwise the example fails:

E:\dev\git2-rs>.\target\debug\blame.exe examples\blame.rs
error: the path 'examples\blame.rs' does not exist in the given tree; class=Tree (14); code=NotFound (-3)

I reused the cfg! conditional from remote.rs, but there might be a better way to do that.

`revparse_single` uses a git object name and not a file path - that means backslashes must be translated to slashes on windows.
Otherwise the example fails:
```
E:\dev\git2-rs>.\target\debug\blame.exe examples\blame.rs
error: the path 'examples\blame.rs' does not exist in the given tree; class=Tree (14); code=NotFound (-3)
```
I reused the `cfg!` conditional from `remote.rs`, but there might be a better way to do that.
format!(
"{}:{}",
commit_id,
path.display().to_string().replace("\\", "/")
Copy link
Contributor

Choose a reason for hiding this comment

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

like I mentioned in gitui-org/gitui#791 I was under the impression that preventing the need for such manual hacks is the reason we have a type like Path in the first place - is display() the problem here? I am using Path::to_str in gitui to get the correct type of path seperators

Copy link
Author

Choose a reason for hiding this comment

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

As I understand it, using revparse_single with the syntax <ref>:<path> implies using /, as it refers to the path of a git object.

blame_file uses a disk path and should (according to libgit2's conventions) accept \ on Windows (in any case git2-rs normalizes with a call to path_to_repo_path).

Normalization is required for this example to accept Windows-style paths.

What I can see as a possible fix:

  • normalize before the revparse call as in this PR (ugly)
  • normalize inside the revparse_single function (weird IMO)
  • remove revparse from this example
  • accept that windows-style paths are rejected in this example (with a comment/help message for the path argument to clear things up)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants