flix / vscode-flix

Apache License 2.0
20 stars 22 forks source link

Incorrect escape of paths #191

Closed JonathanStarup closed 1 year ago

JonathanStarup commented 2 years ago

Continuing from https://github.com/flix/flix/issues/2762, there is a problem (on windows at least) where every special character is escaped even though they shouldn't. Paths like c:Something\here\asd becomes c:Somehting\\here\\asd which is wrong but it works somehow. This becomes an actual error if the path/file contains [ fx.

magnus-madsen commented 1 year ago

@JonathanStarup We believe this is fixed.

Can you confirm if this is not the the case-- we were not able to reproduce...

JonathanStarup commented 1 year ago

this works

Flix 0.34.0 Ready! (Extension: 0.94.0) (Using c:\Users\auxxxxxx\Documents\tmp folder []\flix.jar)

but when the repl is started by vscode it writes

java -jar 'c:\\Users\\au568877\\Documents\\tmp folder []\\flix.jar' repl

and in test.flix when i press run on def main(): String = "asd" it says

flix> :eval main()
-- Resolution Error -------------------------------------------------- <shell>

>> Undefined name 'main'.

2 | unsafe_cast println(main()) as _ \ IO
                        ^^^^
                        name not found
flix>

It seems that its somewhat fixed but not fully

magnus-madsen commented 1 year ago

@mR4smussen Maybe that helps?

mR4smussen commented 1 year ago

Yes thanks, I get the same error now. I will look into this again

mR4smussen commented 1 year ago

I have been playing around with this for a while now. Originally, I was frustrated about being able to run identical commands from vscode and from my terminal getting different results. I think we have two issues here.

  1. When parsing the .jar file from the extension we are escaping characters like [] and have a problem with spaces. I think we can handle this by not escaping characters and having quotation marks around the path.
  2. It seems like the cwd of the main “REPL” has an effect on the compilers ability to find the .flix files and therefor also the methods (hence the “Undefined name 'main'” error above). For instance, my default vscode terminal is PowerShell. So, when I run java -jar 'c:\\...\\flix.jar' repl the cwd is C:\WINDOWS\System32\WindowsPowerShell\v1.0 (Powershell on PATH). But if I make the extension cd into my project folder when I press run before it runs java -jar 'c:\\...\\flix.jar' repl the :eval runs like expected (the first :eval in the image) edit: I just realised that with normal folders (no special characters) my REPL is started in the correct working directory. It seems like no matter what (also outside of vscode) PowerShell has a habit of starting in the default PATH folder if the folder in which it tried to start has a special character… edit 2: I tried changing my default terminal to both the command prompt and an Ubuntu shell and both seemed to handle special characters and spaces in both the folder path and file name. So this issue seems (from my perspective) to only be an issue with PowerShell :)?

image

I’ll try to solve these tomorrow or doing the weekend :)

magnus-madsen commented 1 year ago

@mR4smussen Thanks for the deep dive. Keep me posted.

Unless something simple can be done, it may be an instance "wontfix" (?)

JonathanStarup commented 1 year ago

I think we can handle this by not escaping characters and having quotation marks around the path.

Without too much knowledge of different systems, i think this sounds like the correct way to go since the path is not a string argument to something, but an actual path (e.g. we want \ not \\)

mR4smussen commented 1 year ago

Unless something simple can be done, it may be an instance "wontfix" (?)

I think it’s fair to say that if the users have PowerShell as the default terminal in vscode, then we do not support special characters in the workspace path. If we are still updating this documentation, then perhaps we should remove the “Standby seems to break the connection between VSCode and the Flix language server” and add a note about special characters in the workspace path?

magnus-madsen commented 1 year ago

Unless something simple can be done, it may be an instance "wontfix" (?)

I think it’s fair to say that if the users have PowerShell as the default terminal in vscode, then we do not support special characters in the workspace path. If we are still updating this documentation, then perhaps we should remove the “Standby seems to break the connection between VSCode and the Flix language server” and add a note about special characters in the workspace path?

Roger. I will add a section to that documentation.

magnus-madsen commented 1 year ago

Documentation updated.