Skip to content

Fix opening code via command line using relative paths#3150

Merged
joaomoreno merged 2 commits intomicrosoft:masterfrom
Tyriar:3127_fix_open_relative_path
Feb 22, 2016
Merged

Fix opening code via command line using relative paths#3150
joaomoreno merged 2 commits intomicrosoft:masterfrom
Tyriar:3127_fix_open_relative_path

Conversation

@Tyriar
Copy link
Copy Markdown
Contributor

@Tyriar Tyriar commented Feb 19, 2016

Opening code using code .., code ../.. was opening the wrong directory
since trailing '.' characters were being trimmed by the launch code.

Fixes #3127


@bpasero I'm assuming that the trimming was added for Windows, just wanted to make sure that was the intent. Or can we remove it?

Opening code using `code ..`, `code ../..`  was opening the wrong directory
since trailing '.' characters were being trimmed by the launch code.

Fixes #3127
@Tyriar Tyriar added the bug Issue identified by VS Code Team member as probable bug label Feb 19, 2016
@egamma egamma added this to the Feb 2016 milestone Feb 19, 2016
@egamma egamma assigned joaomoreno and unassigned bpasero Feb 19, 2016
@joaomoreno joaomoreno assigned bpasero and joaomoreno and unassigned joaomoreno and bpasero Feb 22, 2016
@joaomoreno
Copy link
Copy Markdown
Member

I am really afraid of touching this code, and @bpasero is out. No idea why this code is there... makes very little sense to me. E.g. what happens if we tried to open a folder with a dot at the end (folder.)?

Any way, @Tyriar can you give me more details here? Is this because the new launcher script behaves differently? Did you test this on Linux, OSX and Windows?

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Feb 22, 2016

Might be best to wait for @bpasero if you're concerned.

I've only tested on Linux, I don't see why trailing . character would want to be culled on OSX and Linux. AFAIK on Windows somefile. is equivalent to somefile which I guess explains why it's there?

@joaomoreno
Copy link
Copy Markdown
Member

Not being able to open . or .. seems to me to be a much worse proposition than not being able to open somefile. on Windows only... I'd vote for removing that line altogether right now and let @bpasero speak up when he's back.

What do you think?

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Feb 22, 2016

Sounds good to me, I'll just remove it then.

@joaomoreno
Copy link
Copy Markdown
Member

👍

@joaomoreno joaomoreno closed this Feb 22, 2016
@joaomoreno joaomoreno reopened this Feb 22, 2016
joaomoreno added a commit that referenced this pull request Feb 22, 2016
Fix opening code via command line using relative paths
@joaomoreno joaomoreno merged commit fb850a0 into microsoft:master Feb 22, 2016
@bpasero
Copy link
Copy Markdown
Member

bpasero commented Feb 29, 2016

@Tyriar please make sure that #1579 is still fixed after this change. The original issue was that someone managed to save a file called "cd.." which is an invalid file on Windows. Maybe a better fix is to remove trailing dots only when there are no leading dots.

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Feb 29, 2016

@bpasero well I assume that is happening again, not being able to open folders seems far worse though. Maybe something like this would be better:

if windows
  path = normalize(path)
  is path is not a directory
    trim '.' from path

I'll reopen #1579 to fix again.

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Feb 29, 2016

@Tyriar but if the condition is to only trim "." if there is a non-dot before the dot, you would still be able to open "." and ".."?

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Feb 29, 2016

@bpasero normalizing the path beforehand would transform path from c:\foo\bar\.. to c:\foo and c:\foo\bar\. to c:\foo/bar?

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Feb 29, 2016

@Tyriar I would trim all "." from a path on windows, but I see the issue. Why would someone actually type the full path and append ".." at the end? Is that a use case?

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Feb 29, 2016

@bpasero regardless, it should still open the path that's provided, not the path with .'s trimmed. code foo\.. should open ., regardless of whether it's the typical use case. Example usages of where someone would want to do this:

  • Opening via a script
  • If they have typed a path and changed their mind, adding \.. is faster than backspacing

Not doing so will likely result in another issue down the track.

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Feb 29, 2016

@Tyriar makes sense, I am also easy leaving it without the trimming code.

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Feb 29, 2016

I pushed another fix for #1579, I omitted the dir check, not really needed after normalizing. Needs to be verified on Windows.

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Feb 29, 2016

Thanks 👍

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Mar 2, 2016

@Tyriar with your change, opening "." or ".." on windows will result in the unexpected behaviour as before because path.normalize() will not change "." or ".." to anything.

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Mar 2, 2016

@bpasero would joining with cwd because normalizing fix the issue?

path = path.join(process.cwd(), path)

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Mar 2, 2016

Well it might but you can only do so if the path is relative.

@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Issue identified by VS Code Team member as probable bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

code . opens the current directory, code .. also opens the current directory - not the parent

5 participants