Files icon indicating copy to clipboard operation
Files copied to clipboard

Fix: Fixed formatting bug when entering commands into the Address Bar

Open yaira2 opened this issue 1 year ago • 5 comments

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

  • Closes #15447

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Enter git clone https://github.com/files-community/Files into address bar
  2. Confirm the repo is cloned correctly

yaira2 avatar May 22 '24 16:05 yaira2

I think it will be a big hassle to individually blacklist/whitelist each case. My approach would be to pass the original currentInput to LaunchApplicationFromPath instead of the normalized one.

https://github.com/files-community/Files/blob/4b77a5a1c74ffaa5c2bd129fa97bc79009b23590/src/Files.App/ViewModels/UserControls/AddressToolbarViewModel.cs#L782

Then after finding the filename, before LaunchAppAsync (line 827) we can NormalizePathInput if we require.

https://github.com/files-community/Files/blob/4b77a5a1c74ffaa5c2bd129fa97bc79009b23590/src/Files.App/ViewModels/UserControls/AddressToolbarViewModel.cs#L821-L828

FieryRMS avatar May 22 '24 17:05 FieryRMS

https://github.com/files-community/Files/blob/4b77a5a1c74ffaa5c2bd129fa97bc79009b23590/src/Files.App/ViewModels/UserControls/AddressToolbarViewModel.cs#L821-L828

After looking a bit more into this part of the code and doing some testing, it seems like if there is spaces in the path of the application (and there is parameters to the program), the program fails to run eg: C:\Users\FieryRMS\Desktop\Folder 2\test 1.exe param

I propose to additionally check if currentInput starts with "/', then it should get the fileName until the next "/' (or end if it doesn't exist). Then it should work if the command is "C:\Users\FieryRMS\Desktop\Folder 2\test 1.exe" param

FieryRMS avatar May 22 '24 18:05 FieryRMS

I think it will be a big hassle to individually blacklist/whitelist each case. My approach would be to pass the original currentInput to LaunchApplicationFromPath instead of the normalized one.

That's a good approach. Should I disregard https://github.com/files-community/Files/pull/15448#issuecomment-2125469824?

yaira2 avatar May 22 '24 19:05 yaira2

Should I disregard https://github.com/files-community/Files/pull/15448#issuecomment-2125469824?

That bug I mentioned should persist even after the current fix. This issue comes from fileName = trimmedInput.Substring(0, positionOfBlank);. If file path is C:/Folder 1/test 1.exe param then fileName="C:/Folder" instead of "C:/Folder 1/test 1.exe". That's why I proposed that we can check for quotes to calculate the delimiting position, so if the user inputs "C:/Folder 1/test 1.exe" param we are able to correctly execute it.

I suppose I should make a separate issue for this. It seems unrelated to the issue at hand.

FieryRMS avatar May 22 '24 21:05 FieryRMS

Thanks!

yaira2 avatar May 22 '24 21:05 yaira2