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

Click on view file or download binary leads to 404 github page #292

Closed
skaldo opened this issue Nov 1, 2021 · 4 comments
Closed

Click on view file or download binary leads to 404 github page #292

skaldo opened this issue Nov 1, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@skaldo
Copy link

skaldo commented Nov 1, 2021

Bug

When I click on "view file", I expect the file to be opened. Instead I see 404 GitHub page.
The reason is described in Analysis section.

React Native versions

all

Steps to reproduce

See above.

Describe what you expected to happen:

  1. I click on "view file".

  2. I expect the file to be opened.

Analysis

In GitHub url, "Rn" prefix is missing, instead of "RnDiffApp" only "DiffApp" is present. This is due to the fact, that GitHub deliver diffs without prefixed and gitdiff-parser library currently only supports diffs with prefixes.

Solution proposals (cleanest/highest effort -> dirtiest/lowest effort):

  1. Find a way to fetch diffs from GitHub with git prefixes (a/, b/)
  2. Make gitdiff-parser support diffs without prefixes (Add support for diffs created with --no-prefix option ecomfe/gitdiff-parser#21)
  3. Quick&Dirty: Replace DiffApp with RnDiffApp in both DownloadFileButton and ViewFileButton components.
@lucasbento lucasbento added the bug Something isn't working label Nov 2, 2021
@lucasbento
Copy link
Member

Hey, thank you so much for reporting this and proposing fixes!

The PR at gitdiff-parser looks great, I wonder if we could use it with https://github.com/ds300/patch-package, do you think that would work?

This is due to the fact, that GitHub deliver diffs without prefixed and gitdiff-parser library currently only supports diffs with prefixes.

About this, do you have any content that indicated that this was due to a GitHub update? the diffs that we use are actually generated by rn-diff-purge and this started not long ago (hence #291) so it could be that the issue lies there.

@skaldo
Copy link
Author

skaldo commented Nov 2, 2021

Hi @lucasbento,
thanks! I actually thought about providing a PR with patch-package but as no one was complaining yet here I opted in for a "proper" solution 🙂
It took at about a month to merge the last merged PR in gitdiff-parser + we would then need to bump version in react-diff-view before actually being able to do it here.
Looking at all of this, I will provide a PR with patch-package tomorrow. We can remove the patch later once my PR gets propagated through libraries.

About this, do you have any content that indicated that this was due to a GitHub update? the diffs that we use are actually generated by rn-diff-purge and this started not long ago (hence #291) so it could be that the issue lies there.

I strongly think this is the case as "view file" option was working before and rn-diff-purge is actually just "snapshotting" fresh react-native apps, while the diff itself is provided by compare function of GitHub (in this case between tags).

@skaldo
Copy link
Author

skaldo commented Nov 3, 2021

Hi there!
I was just about to write that I will fallback to solution number 3 because it is hard to patch gitdiff-parser becuase it is embedded in react-diff-view which is only published as commonJS package.

But now I see that someone made rn-diff-purge deliver diffs in compatible format (with prefixes). See this one for example.

Would this hidden hero be so kind and tell us what was adjusted to change the diff format?
Feel free to close this 🙂

@pvinis
Copy link
Member

pvinis commented Nov 12, 2022

this seems fixed. if there is a specific example of this failing, please report that.

@pvinis pvinis closed this as completed Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants