Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VS Code fails to open files from relative paths #139845

Open
johnmcfarlane opened this issue Dec 28, 2021 · 13 comments
Open

VS Code fails to open files from relative paths #139845

johnmcfarlane opened this issue Dec 28, 2021 · 13 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug quick-open Quick-open issues (search, commands)
Milestone

Comments

@johnmcfarlane
Copy link

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

  • VS Code Version: 1.63.2 and going back at least a year or two
  • OS Version: at least Ubuntu 18.04 and Windows 10 and 11

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 bpasero added the quick-open Quick-open issues (search, commands) label Dec 28, 2021
@bpasero bpasero assigned TylerLeonhardt and unassigned bpasero Dec 28, 2021
@bpasero
Copy link
Member

bpasero commented Dec 28, 2021

#133438 sounds like it duplicates this issue

@johnmcfarlane
Copy link
Author

#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
Copy link
Member

bpasero commented Dec 28, 2021

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

bpasero commented Dec 28, 2021

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
Copy link
Author

johnmcfarlane commented Dec 28, 2021 via email

@Tyriar
Copy link
Member

Tyriar commented Jan 4, 2022

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

@Tyriar Tyriar removed their assignment Jan 4, 2022
@TylerLeonhardt
Copy link
Member

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

@TylerLeonhardt TylerLeonhardt added the bug Issue identified by VS Code Team member as probable bug label Jan 18, 2022
@TylerLeonhardt TylerLeonhardt added this to the Backlog milestone Jan 18, 2022
@johnmcfarlane
Copy link
Author

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
Copy link

matklad commented Oct 8, 2022

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: rust-lang/rust-analyzer#13367

@SergMariaDB
Copy link

SergMariaDB commented Oct 23, 2023

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
Copy link
Member

bpasero commented Apr 25, 2024

We already do some query normalization here:

let pathNormalized: string;
if (isWindows) {
pathNormalized = original.replace(/\//g, sep); // Help Windows users to search for paths when using slash
} else {
pathNormalized = original.replace(/\\/g, sep); // Help macOS/Linux users to search for paths when using backslash
}

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug quick-open Quick-open issues (search, commands)
Projects
None yet
Development

No branches or pull requests

6 participants