Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Path manipulation using correct OS path class #477

Merged
merged 5 commits into from
Jun 19, 2019

Conversation

hnioche
Copy link
Contributor

@hnioche hnioche commented May 8, 2019

Allow case insensitive path comparison on Windows.
Fixes path generation when local is Windows and remote is Posix.

I had issue debugging a ruby application running in a Linux docker environment using visual studio code on Windows. The issue seems to have started with VSCode 1.30

It was caused by the drive letter part of the workspace directory being lower case while the letter drive part of a file being upper case, preventing the path conversion to happen.

While debugging I also noticed that on Posix, path.join wasn't converting backslashes to slashes, so I changed the join call to handle this case.

  • The build passes
  • TSLint is mostly happy
  • [] Prettier has been run

Allow case insensitive path comparison on Windows.
Fixes path generation when local is Windows and remote is Posix.
@minkir014
Copy link

Did it work successfully with you? I mean you can now remote debug a project on Linux from vscode on windows.

@hnioche
Copy link
Contributor Author

hnioche commented May 8, 2019

Did it work successfully with you? I mean you can now remote debug a project on Linux from vscode on windows.

Yes, I can attach VSCode to the ruby-debug-ide instance in docker, and breakpoints are working now. I haven't tested all scenarios though, I'm gonna test Linux to Docker to make sure I didn't break this part.

Edit: I can confirm it also work correctly on Linux attached to a debugger in docker

@minkir014
Copy link

Please tell me the news because if that worked I'll change my ruby extension source code.

@hnioche
Copy link
Contributor Author

hnioche commented May 8, 2019

Please tell me the news because if that worked I'll change my ruby extension source code.

It does work in both scenarios yes, either having a Windows or Linux running vscode and the ruby debugger running on Linux

@minkir014
Copy link

Have you tried to make the project running on windows and vscode running on Linux?

@hnioche
Copy link
Contributor Author

hnioche commented May 8, 2019

Have you tried to make the project running on windows and vscode running on Linux?

No, the ruby app is a pain to run on Windows, I'll see if I can test with a sample app

@minkir014
Copy link

Try a helloworld app but we want to make sure that there isn't any issues with this.

@hnioche
Copy link
Contributor Author

hnioche commented May 8, 2019

Try a helloworld app but we want to make sure that there isn't any issues with this.

Make sense, I won't have time right now, I'll update you tomorrow on this

@minkir014
Copy link

I'll wait news from you tomorrow.

This fixes attaching vscode from a Posix environment to a debugger runing in Windows
@hnioche
Copy link
Contributor Author

hnioche commented May 8, 2019

Try a helloworld app but we want to make sure that there isn't any issues with this.

I found time to test other scenarios.
windows attached to windows worked out of the box.

I found a bug when attaching from Linux to Windows which is now fixed.

So attaching VSCode from either Windows or Linux to a debugger running in either Windows or Linux works

The only scenario I'm not sure of is if we should always expect to have a remote CWD when the debugger mode is not launch, which I don't think make sense.

@minkir014
Copy link

minkir014 commented May 8, 2019

We can set cwd to {workspaceroot}
I think this will work in all scenarios.

@minkir014
Copy link

minkir014 commented May 8, 2019

Have you crashed into any problems? If you crashed into any problems tell me I can help you.

@hnioche
Copy link
Contributor Author

hnioche commented May 8, 2019

I'm done for the day 🙂
I'll default remote cwd to the local workspace if not defined tomorrow, and i would be done with my changes. Any other thing I should change?

@minkir014
Copy link

No, there isn't. Nice work.

* Use a more accurate way to guess the path OS

* Add missing semicolon

* Default remote workspace to local one

* Run prettier on changes
@hnioche
Copy link
Contributor Author

hnioche commented May 9, 2019

I've applied the last changes.
Now, the remote workspace default to the local one if not defined.

I've also cleaned up a few things (missing semicolon and run Prettier on my changes).

I've tested those last changes again between VSCode running on Windows and Linux against a debugger running on Windows and Linux, all the combinations are working properly.

@minkir014
Copy link

minkir014 commented May 9, 2019

The last thing Have you tried to run a project on Linux and vs code on another Linux.

@hnioche
Copy link
Contributor Author

hnioche commented May 9, 2019

The last thing Have you tried to run a project on Linux and vs code on another Linux.

Yes, I've tried Windows to Windows, Windows to Linux, Linux to Windows and Linux to Linux.

@minkir014
Copy link

Very good. What about OSX?

@hnioche
Copy link
Contributor Author

hnioche commented May 9, 2019

Very good. What about OSX?

I don't have any Apple device to test with

@minkir014
Copy link

OK. very good. so, this pull request didn't run into any problems.
Thanks for your help, @hnioche and nice work.

@DavidGriswoldTeacher
Copy link

I was just banging my head against the wall with this same problem. New ruby-on-rails developer, rolled a docker service to run the app, configured the IDE, but then it seemed to ignore my RemoteWorkspaceRoot. I could see the "C:\Users..." path whenever I added a breakpoint. I just ran this version in testing, and can confirm that it now correctly converts my Windows paths to POSIX paths in the console.

I am still having issues getting breakpoints to catch as of now, but I'm guessing it's another setting somewhere since I'm very new to this. At the very least, the path conversion seems fixed in this version.

@minkir014
Copy link

@DavidGriswoldTeacher Could you post your launch.json to see where is the problem?

@DavidGriswoldTeacher
Copy link

DavidGriswoldTeacher commented May 13, 2019 via email

@minkir014
Copy link

Please, provide your launch.json I knew what is the problem in it (launch.json) but I want to be sure. Please provide it to see if there is another problem we can solve.

@rubyide rubyide locked as off-topic and limited conversation to collaborators May 13, 2019
@wingrunr21
Copy link
Collaborator

@hnioche thank you for your work. I'll take a look at this soon!

@wingrunr21 wingrunr21 merged commit f3d049c into rubyide:master Jun 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants