microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.66k stars 28.68k forks source link

VS Code fails to open files from relative paths #139845

Open johnmcfarlane opened 2 years ago

johnmcfarlane commented 2 years ago

Does this issue occur when all extensions are disabled?: Yes

Steps to Reproduce:

  1. Open a file which contains a parent, ../, directory from either the "Go to File..." (Ctrl-P) option or click on such a path as part of compiler output.
  2. Observe that the file exists but that VS Code reports "No matching results"
  3. Normalise the path so that the ../ is gone. The file can now be opened by VS Code, even though the physical path is unchanged.

Note that I opened an issue as a feature request but have since determined that this is plain broken. (IIRC it works in other tools such as CLion.) Despite a request to reconsider the status of the issue, it remains closed and various open file features remain broken.

Thanks

bpasero commented 2 years ago

https://github.com/microsoft/vscode/issues/133438 sounds like it duplicates this issue

johnmcfarlane commented 2 years ago

133438 is a feature request about ordering of quick-open files based on relative path of file in focus in the editor. This issue is a bug about failure to normalise paths with relative directories in them (i.e. . and ..).

bpasero commented 2 years ago

Right, and I think this is more usual to happen when opening links from the integrated terminal, so adding Daniel. I think before opening quick pick, maybe we should normalize paths if possible?

johnmcfarlane commented 2 years ago

An example might help. Consider two files:

// repo-dir/sub-dir1/file1.h
#error
// repo-dir/sub-dir2/file2.cpp
#include "../sub-dir1/file1.h"

At least two problems arise when VS Code is confronted with this structure:

  1. "Go to File..." facility fails when ../sub-dir1/file1.h (or, say, sub-dir2/../sub-dir1/file1.h:2:2) is entered because it does not attempt file path normalisation. (Here's an example of a path normalisation routine.)
  2. When file2.cpp is compiled, the compiler emits a diagnostic when it parses file1.h and this is displayed by some/all compilers as a relative path, e.g.:
    In file included from /home/john/ws/vscode-139845/sub-dir2/file2.cpp:2:
    /home/john/ws/vscode-139845/sub-dir2/../sub-dir1/file1.h:2:2: error: #error 
       2 | #error
         |  ^~~~~

    Clicking on this path in the output window fails to cause the file to be opened - again because normalisation does not occur.

HTH, John

johnmcfarlane commented 2 years ago

I think before opening quick pick, maybe we should normalize paths if possible?

@bpasero I wouldn't like to say whether there are any gotchas here. You might want to try twice: once with the naked path and if unsuccessful, then try normalised. I don't know if this is more or less messy though.

bpasero commented 2 years ago

But if we were to resolve a path like ../sub-dir1/file1.h we need something to resolve it against. Best we can probably do is to take the workspace root in this case, though with multi-root workspaces you can end up having many.

I think for paths like foo/../bar.txt the solution is quite straight forward.

johnmcfarlane commented 2 years ago

I think eating all leading relative dirs here would produce decent results. In this case, sub-dir1/file1.h. And I'm not sure it's the common case anyway.

On Tue 28 Dec 2021, 15:08 Benjamin Pasero, @.***> wrote:

But if we were to resolve a path like ../sub-dir1/file1.h we need something to resolve it against. Best we can probably do is to take the workspace root in this case, though with multi-root workspaces you can end up having many.

— Reply to this email directly, view it on GitHub https://github.com/microsoft/vscode/issues/139845#issuecomment-1002149793, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFTGN5DGNXCZLAXHBX25JDUTHHHTANCNFSM5K4CVPGQ . You are receiving this because you authored the thread.Message ID: @.***>

Tyriar commented 2 years ago

No terminal issue here, I created a similar issue in the past but we now workaround it by just removing the relative prefix:

https://github.com/Microsoft/vscode/blob/b3b82c054077df8128e4af45bac1b019db833f69/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts#L138-L140

It might make sense to roll that into go to file, but I don't know if there are other implications with that

TylerLeonhardt commented 2 years ago

Yeah I like the idea of eating the reading relative dirs.

johnmcfarlane commented 2 years ago

Note that they are always an indication of an incomplete path, i.e. one in which a directory name followed by a .. was sliced down the middle. Hence it should be safe to discount the .. as unhelpful in guessing the intended ultimate file or directory.

Only if that assumption is somehow false would eating .. cause a regression.

matklad commented 1 year ago

This is a relatively big problem for Rust: when rustc prints an error, it is usually relative to Cargo's workspace root, and cargo's workspace root is quite often not the same as Vs Code workspace root.

A nice way to solve this would be some kind of an API for extensions to resolve paths -- at least in the case of cargo, correct resolution of realtive paths requires language-specific knowledge which is available in the extension.

Actually, I think I've solved that: problem matchers support variable substitiution, and variable substitution supports running custom commands: https://github.com/rust-lang/rust-analyzer/pull/13367

SergMariaDB commented 11 months ago

This seem to must be solved by checking for files that ended with relative pattern excluding any prefixed '../' because that would be too tricky to know current directory of a script/application during a line output.

bpasero commented 4 months ago

We already do some query normalization here:

https://github.com/microsoft/vscode/blob/34e1f76c491c37fe33f285ab6508b9ea3d235cec/src/vs/base/common/fuzzyScorer.ts#L894-L899

So maybe a logic to drop .. and . could be added there. I wonder if a path.normalize would suffice, that should take care of it. The method is used in other contexts though.

stangelandm1 commented 2 months ago

To be clear, this didn't used to be a problem. I'm not sure when it got introduced though.

My use case is when trying to click paths in the integrated terminal. My build runs from a sub folder, so any time it reports issues with source code, it's all prefixed with a few parent folder paths (../..).

Has there been any progress?

vladlabs commented 1 month ago

Hi. I have the same issue while running some tests in rust projects. It outputs some paths to files in a relative format like ./src/tests/yourtest.rs:50

And VS code failing to open this file even if this path is relative to the root folder of the VS project. It just output search bar with the relative URI. After manual prefix "./" removal VSCode is able to open file