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

Generate new URL string for each entry in readmeNames #151

Closed
wants to merge 2 commits into from

Conversation

angadgill92
Copy link

@angadgill92 angadgill92 commented Jul 17, 2020

The code in github.go and gitlab.go mutates the same URL path
making anything but the first attempt incorrect. This means the project
at the moment only works for repositories that have README.md present.

I added a debug log to findGithubREADME and tried to get the readme
of one of my own projects (it has a readme.md)

here is the output:

$ go run . github.com/angadgill92/clean
TRYING PATH  /angadgill92/clean/master/README.md
TRYING PATH  /angadgill92/clean/master/README.md/master/README
Error: can't find README in GitHub repository
Usage:
  glow SOURCE [flags]

Flags:
  -h, --help           help for glow
  -p, --pager          display with pager
  -s, --style string   style name or JSON path (default "auto")
      --version        version for glow
  -w, --width uint     word-wrap at width

exit status 255

The reason for this is that we're doing

   v := u
   v.Path += "/master/" + r

and on every iteration of the loop it adds "/master/" + r to
whatever URL was generated in the last iteration. This makes
every URL generated by glow after the first one invalid causing
the error.

Also fixes #134 🙂

The code in `github.go` and `gitlab.go` mutates the same URL path
making anything but the first attempt incorrect. This means the project
at the moment only works for repositories that have `README.md` present.

I added a debug log to `findGithubREADME` and tried to get the readme
of one of my own projects (it has a readme.md)

here is the output:

```shell
$ go run . github.com/angadgill92/glow
TRYING PATH  /angadgill92/clean/master/README.md
TRYING PATH  /angadgill92/clean/master/README.md/master/README
Error: can't find README in GitHub repository
Usage:
  glow SOURCE [flags]

Flags:
  -h, --help           help for glow
  -p, --pager          display with pager
  -s, --style string   style name or JSON path (default "auto")
      --version        version for glow
  -w, --width uint     word-wrap at width

exit status 255
```

The reason for this is that we're doing
```go
   v := u
   v.Path += "/master/" + r
```

and on every iteration of the loop it adds `"/master/" + r` to
whatever URL was generated in the last iteration. This makes
every URL generated by `glow` after the first one invalid causing
the error.
@angadgill92
Copy link
Author

Hi, just checking if there are any plans to merge this? If there are any issues please let me know? 🙏

@muesli
Copy link
Contributor

muesli commented Aug 25, 2020

Hey @angadgill92! Sorry for the late response. You're certainly right, we should do better here. I wonder if there's a way to find a file (entirely) case-insensitively - like we do for local files - but I fear there's no way that doesn't involve the GitHub or GitLab APIs. Otherwise the next corner-case would be a Readme.md, then a readme.MD, and so on...

@angadgill92
Copy link
Author

Hey, no worries 🙂 You make a valid point, I wonder if there's a way to list the contents of the root directory of a GitHub/Gitlab repo, without cloning it 🤔. If we could get a list of root directories via some API (as you rightly pointed out), we could pattern match the paths case insensitively and pick the correct path for the readme, and then make a single call to fetch it instead of trying to brute force.

Are you apprehensive about using GitHub API/Gitlab API? If it's okay with you I'd like to find some time over this weekend to explore this, and update the PR.

@muesli
Copy link
Contributor

muesli commented Aug 25, 2020

Since we already have GitHub / GitLab specific implementations I'm not really opposed, I was just hoping to find something a bit more generic, that would also support the likes of a self-hosted Gitea (or similar). I don't think that's easily possible though.

Definitely looking forward to what you come up with!

@cristiand391
Copy link
Contributor

Hi, is someone already working on this?

I have a very simple implementation using the github api and would like to adapt it to glow if possible.

@muesli
Copy link
Contributor

muesli commented Sep 28, 2020

By all means @cristiand391, go ahead. Curious to see what you come up with!

@caarlos0
Copy link
Member

caarlos0 commented Jul 3, 2024

I think this was fixed on master already, #450

thanks, and sorry for the delay

@caarlos0 caarlos0 closed this Jul 3, 2024
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.

Make it aware of lowercase readme by default
4 participants