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

Intercept and convert \\wsl.localhost\ to \\wsl$\ paths #10908

Merged
merged 1 commit into from Apr 23, 2023

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Apr 22, 2023

Resolves #10025
Resolves #10815

Test methodology

  • manual
  • unit tests

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


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

@RussKie RussKie added this to the 4.1 milestone Apr 22, 2023
@ghost ghost assigned RussKie Apr 22, 2023
@RussKie RussKie requested a review from gerhardol April 22, 2023 14:38
@vbjay
Copy link
Contributor

vbjay commented Apr 22, 2023

Should this be an exe.config thing? Allow for the matches to be defined and the replacement.

Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

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

just doc typos, have not run

GitCommands/PathUtil.cs Outdated Show resolved Hide resolved
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

Have run very briefly.
I believe this is all that is needed, took a quick look at the implementation.

This will display recent (and current) repos opened as wsl.localhost as wsl$. From a usability view, that is better as the recents are not duplicated. Users may be confused though.
(It is also much simpler to implement it this way.)

I will update the doc.

This feature was something I considered to, as wslpath returns wsl.locahost and then you get the extra item.
I decided to not delay 4.1 for this, but have no problem accepting changes from someone else.

@RussKie
Copy link
Member Author

RussKie commented Apr 23, 2023

Should this be an exe.config thing? Allow for the matches to be defined and the replacement.

I'm not sure what you mean, sorry.

@RussKie
Copy link
Member Author

RussKie commented Apr 23, 2023

Users may be confused though.

Perhaps, but looks like users get confused by "wsl.localhost" more.

GitExtensions/Program.cs Outdated Show resolved Hide resolved
@RussKie RussKie merged commit 5c2980b into gitextensions:release/4.1 Apr 23, 2023
3 checks passed
@RussKie RussKie deleted the intercept_wsl.localhost branch April 23, 2023 09:24
@vbjay
Copy link
Contributor

vbjay commented Apr 23, 2023

Should this be an exe.config thing? Allow for the matches to be defined and the replacement.

I'm not sure what you mean, sorry.

If wsl decides to change aliases or if user can change it to something else. Thinking if they needs to fix we can at least tell them to change the config file to allow it to work till we make a release.

@RussKie
Copy link
Member Author

RussKie commented Apr 23, 2023

Should this be an exe.config thing? Allow for the matches to be defined and the replacement.

I'm not sure what you mean, sorry.

If wsl decides to change aliases or if user can change it to something else. Thinking if they needs to fix we can at least tell them to change the config file to allow it to work till we make a release.

These are Windows defined, so I don't expect these to change often.

@gerhardol
Copy link
Member

Users may be confused though.

Perhaps, but looks like users get confused by "wsl.localhost" more.

There will be issues with users claiming that wsl.localhost is the new name and that it should be used instead of the old name, or that the entered name is used.
I prefer this solution, just wanted to point that out. i prefer this way:

  • A solution where the entered name is used will be more complex and may get two entries in the recent list.
  • We could translate to wsl.localhost instead, for users with wsl 1.0 or later. That will require a check for older version (either translate differently or use Windows Git). wsl$ is a shorter prefix too.

These are Windows defined, so I don't expect these to change often.

wsl$ was changed just because it contains illegal characters, requiring special handling. I would not expect this to be changed again.
There are some special handling to wsl handling, like file notification is not working. So wsl is a special handling.

*Nix users (with wine or so) may need a similar "mode, triggered on something else.

@andrej-schaefer
Copy link

I see a problem with this solution for the following use case:
Use WSL for DEV and runtime, for this purpose code is in the WSL file system
Use wsl.localhost for gitExtensions due to use putty and configure different ssh keys per project / customer with a passphrase

With the new solution it change to wsl and does not take the configured ssh in gitExtensions any more but redirects to default ssh in WSL

Workaround is to downgrade of gitExtensions or move repos to windows
Easy solution to handle multiple ssh files on linux i havent found so far, maybe someone have a solution for this problem
Propose is complex, maybe select in the repository selection or some kind of configuration
Hope this comment is ok, if not, please let me know :)

WSL: WSL 2 with latet ubuntu
Windows 11

@gerhardol
Copy link
Member

Git repos mapped to a drive letter will not use the special WSL handling but Windows Git

From https://git-extensions-documentation.readthedocs.io/en/release-4.1/settings.html#wsl-git-notes
(will modify the formatting)

There are too many settings in GE already, I would like to avoid more (there are some hidden settings for WSL, not related to this though).

Originally, I wanted to have wsl.localhost separated as you describe, but Windows insists on wsl.localhost in most conversions so it it inconvenient...

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.

Use native WSL Git executables also for \\wsl.localhost as for \\wsl$ wsl2 fatal error on windows 11
5 participants