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

Improve determination of the GitExt directory (fix of issue #7587) #7591

Merged
merged 1 commit into from
Jan 2, 2020

Conversation

c3er
Copy link
Contributor

@c3er c3er commented Jan 2, 2020

Fixes

Issue #7587.

Proposed changes

  • Change determination of GitExt loacation from new Uri(assembly.CodeBase).LocalPath to assembly.Location
  • Added some test cases. Though they run also green without the proposed change.

Before

If the path to the GitExt repository contains a hash sign "#":

  • Unit test Default_values_are_specified_in_invariant_theme fails.
  • Debugging from Visual Studio starts with error message "Failed to read theme"

After

Mentioned errors are fixed.

Test methodology

  • Made sure related errors disappeared
  • Added some test cases
  • Ran all existing unit tests to make sure they are still green
  • Searched for other occurrences of new Uri... bit did not found anything that causes similar bugs

Test environment(s)

  • GIT 2.24.1.windows.2
  • Windows 10 1909

My actual path to the local Gitext repository is

D:\eigene dateien\devel\bsp\c#\gitextensions

✒️ I contribute this code under The Developer Certificate of Origin.

If the GitExt repo directory contains a hash sign "#", determination of
the actual path failed.

This fixes GitExtensions issue gitextensions#7587.
@ghost ghost assigned c3er Jan 2, 2020
@codecov
Copy link

codecov bot commented Jan 2, 2020

Codecov Report

Merging #7591 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #7591      +/-   ##
==========================================
- Coverage   47.77%   47.76%   -0.01%     
==========================================
  Files         831      831              
  Lines       58640    58640              
  Branches     7562     7562              
==========================================
- Hits        28016    28012       -4     
- Misses      29174    29176       +2     
- Partials     1450     1452       +2
Flag Coverage Δ
#production 36.86% <100%> (-0.01%) ⬇️
#tests 97.59% <ø> (ø) ⬆️

@RussKie RussKie merged commit 02594d1 into gitextensions:master Jan 2, 2020
@ghost ghost added this to the 4.0 milestone Jan 2, 2020
@RussKie
Copy link
Member

RussKie commented Jan 2, 2020

@c3er I completely missed it, but we need your sign-off in contributors.txt

@gerhardol
Copy link
Member

Also note
#7587 (comment)

URI.Local need to be changed in several other situations. Rules for URI is not the same as for the local file system

@c3er
Copy link
Contributor Author

c3er commented Jan 2, 2020

@gerhardol I searched the code for new Uri and other occurrences that involve local path seem not to be affected by truncation. I observed that if the path has the format

file:///C:/some#/dir/file.txt

the path will e truncated like in the line var path = new Uri(assembly.CodeBase).LocalPath;. But if the path has not the file protocol prefix, the path will not be truncated by the Uri constructor. It could be more clean to replace all Uri constructor calls to local paths with more appropriate seeming methods but I hesitated because I don't know how to find and test all related side effects.

@RussKie Shall I make a new pull request for this or how should it be handled?

@gerhardol
Copy link
Member

I hesitated because I don't know how to find and test all related side effects.

OK

Shall I make a new pull request for this or how should it be handled?

Yes

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.

3 participants