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

Fix improper handling of file paths with bytes > 0x80 #10

Closed
wants to merge 3 commits into from
Closed

Fix improper handling of file paths with bytes > 0x80 #10

wants to merge 3 commits into from

Conversation

slnc
Copy link
Contributor

@slnc slnc commented Apr 22, 2022

Git log's default behavior is to quote strings with special characters but that isn't what gitmap's clients like Hugo expect so when they interact with gitmap, they send unquoted file paths and get no git info (see gohugoio/hugo#9810 for an example).

h/t @jmooring for the fix and @bep for the help, I just packaged their ideas into this pull request.

Copy link
Contributor Author

@slnc slnc left a comment

Choose a reason for hiding this comment

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

If I don't add the showSignatures bit I get a merge conflict with bep:master warning, but I can't figure out where it's coming from. It's my first github pull request, sorry if I'm doing something dumb.

@jmooring
Copy link

I'm not sure what you did, but the typical PR workflow is:

  1. Fork on GitHub
  2. Clone locally
  3. Create new branch
  4. Make changes
  5. Test, test, test
  6. Commit changes to your new branch
  7. Push your new branch to your fork (origin)
  8. Create PR from the new branch on your fork

@slnc
Copy link
Contributor Author

slnc commented Apr 22, 2022

I missed the creating a new branch part, sorry, I'm going to redo the PR, thanks.

Another basic question: how do you run the existing gitmap_test.go tests with the current Go version (1.18.1)? I can't get it to run unless I create a go.mod file and then run go test -v gitmap.

Thanks

@jmooring
Copy link

I'm not sure why this module doesn't have go.mod/sum files. Maybe it was created before Go Modules was a thing.
Need guidance from @bep on this.

@slnc
Copy link
Contributor Author

slnc commented Apr 22, 2022

Will tentatively send the PR with a mod.go in it.

@slnc slnc closed this Apr 22, 2022
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