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

better specify the cmdline interpreter #11833

Closed
wants to merge 4 commits into from

Conversation

xomx
Copy link
Contributor

@xomx xomx commented Jun 23, 2022

Fixes #11818 .

This solves a possible confusion between the cmdline interpreter and the possible folder named "cmd"' for the ShellExecute WINAPI.

This solves a possible confusion in between the cmdline interpreter and the possible folder named "cmd"' for the ShellExecute WINAPI.
@dail8859
Copy link
Contributor

Kind of a dumb question, but what about if there is a cmd.exe file in the local directory? Is this situation even avoidable?

https://docs.microsoft.com/en-us/windows/win32/shell/app-registration?redirectedfrom=MSDN#finding-an-application-executable

The file is sought in the following locations:

    The current working directory.
    The Windows directory only (no subdirectories are searched).
    The Windows\System32 directory.
    Directories listed in the PATH environment variable.
    Recommended: HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths

@xomx
Copy link
Contributor Author

xomx commented Jun 24, 2022

@dail8859

Kind of a dumb question, but what about if there is a cmd.exe file in the local directory?

It is a legitimate question, ShellExecute then really tries to open any file named "cmd.exe" from the dir selected (e.g. from the "faw_test" dir example mentioned in the issue). So if somebody copies there e.g. the Windows "notepad.exe" file, renames it to the "cmd.exe", then the Windows Notepad will be opened instead of the command prompt requested.

Is this situation even avoidable?

Yes, when we use "%WINDIR%\System32\cmd.exe" instead of my current "cmd.exe". I tried that and it works ok.

But there is still something I need to understand - during my testing I have found that there is a N++ GUIConfig for this cmdline interpreter, which I cannot find in the N++ GUI or in the correspondent "config.xml": commandLineInterpreter

And at the next line is a hard-coded comparison to TEXT("cmd"), so it needs the same adjustment. I will probably create a common define for it to be used at both places.

@dail8859
Copy link
Contributor

@xomx

But there is still something I need to understand

Luckily the documentation is kept up really well.

https://npp-user-manual.org/docs/preferences/#preferences-for-advanced-users

It is for advanced users that want to manually change the interpreter in the config file themselves.

@vinsworldcom
Copy link

It is for advanced users that want to manually change the interpreter in the config file themselves.

I do that:

C:\usr\bin\npp64\config.xml
115:         <GUIConfig name="commandLineInterpreter">powershell</GUIConfig>

Cheersl

@MarkusBodensee
Copy link
Contributor

Kind of a dumb question, but what about if there is a cmd.exe file in the local directory? Is this situation even avoidable?

And the next one... What happens now with a folder named cmd.exe in the local directory. I think same happens like mentioned in the issue, folder opens in explorer.

@xomx
Copy link
Contributor Author

xomx commented Jun 24, 2022

@MarkusBodensee

Yes, the "cmd.exe" folder will be opened in the explorer.
It is interesting that when one creates both "cmd" and "cmd.exe" subfolders, the explorer opens the "cmd.exe" one.

My latest version ("%WINDIR%\System32\cmd.exe") solves even that (I checked).

@@ -46,6 +46,7 @@

#endif

#define CMD_INTERPRETER TEXT("%WINDIR%\\System32\\cmd.exe")
Copy link
Contributor

Choose a reason for hiding this comment

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

Better use constants in C++, instead of defines ?

const TCHAR cmdInterpreter[] = TEXT("%WINDIR%\\System32\\cmd.exe");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is equivalent (the above #define... will become the same const TCHAR*).

@tarkett2018
Copy link

@xomx

Yes, when we use "%WINDIR%\System32\cmd.exe" instead of my current "cmd.exe". I tried that and it works ok.

Only a suggestion: You can also use the environment variable %COMSPEC%. Then you get directly the command line interpreter which can be different in mixed x86/x64 systems.

@xomx
Copy link
Contributor Author

xomx commented Jun 25, 2022

@tarkett2018

Nice, that is even better, it works ok. (x86/x64 was not a problem before - the WinOS redirector handles that for 32/64-bit apps)

I hope this is the final version, I will be computer-free until next weekend.

@donho donho self-assigned this Jun 27, 2022
@donho donho added the accepted label Jun 27, 2022
@donho donho closed this in 7917d0d Jun 27, 2022
@xomx xomx deleted the patch-5 branch July 3, 2022 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FAW - "CMD here" does not work if there is a subdirectory named cmd
6 participants