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

Allow to set custom installation paths for supported diff tools #310

Closed
ldeluigi opened this issue Dec 14, 2022 · 14 comments · Fixed by #314
Closed

Allow to set custom installation paths for supported diff tools #310

ldeluigi opened this issue Dec 14, 2022 · 14 comments · Fixed by #314

Comments

@ldeluigi
Copy link
Contributor

Is the feature request related to a problem

I have a custom installation folder for both VS Code and Visual studio. The search paths for these diff tools don't match with the ones hard-coded inside DiffEngine.

Describe the solution

One should be able to set a custom path for diff tools, for example with the help of environment variables.
For example: DiffEngine_<DiffToolName>_Path = D:\My\Folder\

Describe alternatives considered

As alternative, consider reading a configuration file in the home directory of the user or in the project folder.

Additional context

If necessary ask me.

@SimonCropp
Copy link
Member

you can add VS Code and Visual studio to you PATH variable

@ldeluigi
Copy link
Contributor Author

you can add VS Code and Visual studio to you PATH variable

I did, but this is not enough. DiffEngine wants a specific executable file name and path.

@SimonCropp
Copy link
Member

I did, but this is not enough. DiffEngine wants a specific executable file name and path.

that sounds like a bug. can you submit a failing test as a Pull Request. here is a test you can look at as a basis https://github.com/VerifyTests/DiffEngine/blob/main/src/DiffEngine.Tests/OsSettingsResolverTest.cs#L21

@ldeluigi
Copy link
Contributor Author

ldeluigi commented Dec 16, 2022

I don't need to make a PR. The problem is simple and can be seen from the code. Look at this:

Windows: new(
"code.exe",
launchArguments,
@"%LocalAppData%\Programs\Microsoft VS Code\",
@"%ProgramFiles%\Microsoft VS Code\"),

On Windows you are looking for a code.exe file in those directory or in the path. But in my Windows 10 system, visual studio code has a root directory with a Code.exe file, a bin directory with a code, code.cmd and a code-tunnel.exe file, which are all kind of equivalent but don't match code.exe (I think). The full path of my VS Code installation is F:\Programs\Microsoft VS Code.

General thoughts

It's not always a good idea to search the entire PATH of the system for a file named code.exe because any program can have such file and you are calling god knows what program that's named like that when you do so. Adding a custom, explicit manual configuration option would prevent such mistakes and give the user control over what's being done by DiffEngine in our systems. I've already set up a DiffEngine_ToolOrder variable to control priority, but none of the implementations I listed worked properly until I monkeypatched vs code by creating a code.exe file in the bin folder which is a copy of code-tunnel.exe. I'd also appreciate if DiffEngine prompted the errors encountered in finding the implementations I listed (VisualStudioCode,VisualStudio), but it's not critical.

@SimonCropp
Copy link
Member

given on your machine F:\Programs\Microsoft VS Code is in path and Code.exe then this code should find it https://github.com/VerifyTests/DiffEngine/blob/main/src/DiffEngine/OsSettingsResolver.cs#L109

@SimonCropp
Copy link
Member

closing this for now. if u can share a failing unit test as a PR we can look at it again

@ldeluigi
Copy link
Contributor Author

This works:
image

This doesn't:
image

@ldeluigi
Copy link
Contributor Author

Another way to make it work is to put the entire Vs Code installation folder in the PATH, isntead of just the bin folder

@ldeluigi
Copy link
Contributor Author

In that case it finds the Code.exe file

@ldeluigi
Copy link
Contributor Author

@SimonCropp can you make it look for either a Code.exe or a code file?

@SimonCropp
Copy link
Member

Another way to make it work is to put the entire Vs Code installation folder in the PATH, isntead of just the bin folder
In that case it finds the Code.exe file

yep. and isnt that the fix for you?

@ldeluigi
Copy link
Contributor Author

Another way to make it work is to put the entire Vs Code installation folder in the PATH, isntead of just the bin folder
In that case it finds the Code.exe file

yep. and isnt that the fix for you?

It would be, but I'd rather avoid adding that folder to the path given that it's necessary just for DiffEngine.

@ldeluigi
Copy link
Contributor Author

I want to stress the fact that forcing a parsing of the system PATH it's not a very good practice in general. One should be able to configure exact specific paths to executables of their diff programs. This avoids mistakes and confusion on which executable the engine is running in case of multiple installation of the same program and some other edge cases...

@SimonCropp
Copy link
Member

configure exact specific paths to executables of their diff programs

if u want to do a pull request for that, i am happy to consider it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants