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

Git History open diff editor have error title #5956

Closed
502647092 opened this issue Aug 16, 2019 · 7 comments · Fixed by #5971
Closed

Git History open diff editor have error title #5956

502647092 opened this issue Aug 16, 2019 · 7 comments · Fixed by #5971
Labels
bug bugs found in the application critical critical bugs / problems git issues related to git help wanted issues meant to be picked up, require help

Comments

@502647092
Copy link
Contributor

Description

image

Reproduction Steps

  • open git history
  • open commit
  • open diff editor

OS and Theia version:
git-commit: 9105c43

Diagnostics:

@akosyakov akosyakov added bug bugs found in the application git issues related to git help wanted issues meant to be picked up, require help critical critical bugs / problems labels Aug 16, 2019
@akosyakov
Copy link
Member

@vince-fugnitto maybe we introduced it by 9105c43#diff-030d37e8ae5f19ecc3c08047f819e016R100 not sure

@502647092 is there a change that you can have a look into it?

@vince-fugnitto
Copy link
Member

Checking out master at e8a29ca, I can confirm that the issue is also present.

@akosyakov
Copy link
Member

ok, someone has to figure out what is wrong, we also changed NavigatableWidgetOpenHandler, maybe it is the reason

I can have look earlier somewhere next week, if someone can before please do

@vince-fugnitto
Copy link
Member

ok, someone has to figure out what is wrong, we also changed NavigatableWidgetOpenHandler, maybe it is the reason

It has to do with the changes from #5918.
Removing the call normalizePath() fixes the issue.

@akosyakov
Copy link
Member

@a1994846931931 Would you have change to have a look at #5956 (comment)? It seems we missed some cases.

@a1994846931931
Copy link
Contributor

@a1994846931931 Would you have change to have a look at #5956 (comment)? It seems we missed some cases.

@akosyakov Sure, I'll take a look at it soon. Maybe tomorrow morning, since it's late at night here and I happen to be on a train.

@a1994846931931
Copy link
Contributor

The problem is caused by that empty path '' will be normalized as . This is also the way how Node.JS path handle it. So it isn't a problem, until we call URI.normalizePath() on URI whose scheme is not file. A PR will come soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application critical critical bugs / problems git issues related to git help wanted issues meant to be picked up, require help
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants