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

Link detection fails around punctuation characters in Output windows #191398

Closed
kriswuollett opened this issue Aug 26, 2023 · 6 comments
Closed
Assignees
Labels
info-needed Issue requires more information from poster *out-of-scope Posted issue is not in scope of VS Code

Comments

@kriswuollett
Copy link

kriswuollett commented Aug 26, 2023

I'd think that any quote-like character that appears before a detected URL in places like the Terminal or Output windows should be trimmed from the end of the URL as a heuristic. Because for example, currently the trailing backtick is included in the file URL, so an error of "Cargo.toml`" is not will appear in a dialog upon click following:

Caused by:
  failed to parse manifest at `/Users/kris/Code/some-project/Cargo.toml`

See rust-lang/rust-analyzer#15518

@kriswuollett
Copy link
Author

Also noticed absolute file paths that have a trailing comma because it is middle of a sentence also doesn't work. Even if this like this are legal paths or URLs, it would be more likely wrong contextually if it was sentence punctuation. A bare question mark also probably wouldn't hurt being dropped from contextual links since that would be an empty query anyways.

@mjbvz mjbvz assigned Tyriar and unassigned mjbvz Aug 28, 2023
@Tyriar
Copy link
Member

Tyriar commented Aug 28, 2023

This should already be working:

Screenshot 2023-08-28 at 9 14 12 AM

The panic links you showed at the bottom also should work:

Screenshot 2023-08-28 at 9 15 52 AM Screenshot 2023-08-28 at 9 16 27 AM

It's not clear if the first one is relative to the workspace or some rust internals, if it's the latter a link provider could be implemented in the rust extension to handle this.

Any more details on reproducing this? Could you see if this happens in Insiders also? https://code.visualstudio.com/insiders

If it still does not work on Insiders you can enable the trace log level via the command palette, hover the lines in question and then copy the Output > Terminal output channel.

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Aug 28, 2023
@kriswuollett
Copy link
Author

Any more details on reproducing this? Could you see if this happens in Insiders also? https://code.visualstudio.com/insiders

If it still does not work on Insiders you can enable the trace log level via the command palette, hover the lines in question and then copy the Output > Terminal output channel.

I'll try and get back soon with more info on how to reproduce. However your tests above are with a user shell Terminal. My examples were from things like the Rust Analyzer Client and Rust Analyzer Language Server output windows. I assume they share a lot of code, but technically not the same thing. I think it may be as easy as checking out a Rust open-source project that uses a Rust workspace, and just changing one of the sub-crates to specify a crate dependency that does not exist to get that cannot read Cargo.toml error.

@Tyriar
Copy link
Member

Tyriar commented Aug 29, 2023

@kriswuollett oh do you mean they're in the output panel, not the terminal panel? The output panel is totally separate and lacks a lot of the features of the terminal.

@kriswuollett
Copy link
Author

@kriswuollett oh do you mean they're in the output panel, not the terminal panel? The output panel is totally separate and lacks a lot of the features of the terminal.

Yes, let me update the title since I made an assumption that the Terminal would behave the same where I saw the issue in the Output windows.

Would it be rather straightforward for the Output windows to be brought to parity with the Terminal windows w.r.t. link detection?

Would it be something like the following such that the responsibility can be taken over by the extension owner of the output window(s):

registerLinkProvider(linkProvider: ITerminalExternalLinkProvider): IDisposable {
const disposables: IDisposable[] = [];
this._linkProviders.add(linkProvider);
this._onDidAddLinkProvider.fire(linkProvider);
return {
dispose: () => {
for (const disposable of disposables) {
disposable.dispose();
}
this._linkProviders.delete(linkProvider);
this._onDidRemoveLinkProvider.fire(linkProvider);
}
};
}

@kriswuollett kriswuollett changed the title Default link detection should use quoting heuristic to avoid trailing quote character Link detection fails around punctuation characters in Output windows Aug 30, 2023
@Tyriar
Copy link
Member

Tyriar commented Aug 30, 2023

@kriswuollett it would be quite tough unfortunately as they're completely separate codebases (output based on monaco, terminal based on xterm.js). Here's the out of scope request that tracked what you're asking for #107314, and some more link requests #163778, #149153.

If you need good link detection you need to use a terminal, something an extension can do which might be relevant is to create a background terminal and then show it on command:

vscode/src/vscode-dts/vscode.d.ts

Lines 11012 to 11019 in cd7388f

/**
* When enabled the terminal will run the process as normal but not be surfaced to the user
* until `Terminal.show` is called. The typical usage for this is when you need to run
* something that may need interactivity but only want to tell the user about it when
* interaction is needed. Note that the terminals will still be exposed to all extensions
* as normal.
*/
hideFromUser?: boolean;

Or just buffer the data and delay creating the terminal all together until asked.

Closing this out as it's up to an extension to route the content to the terminal for good link support.

@Tyriar Tyriar closed this as completed Aug 30, 2023
@Tyriar Tyriar added the *out-of-scope Posted issue is not in scope of VS Code label Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed Issue requires more information from poster *out-of-scope Posted issue is not in scope of VS Code
Projects
None yet
Development

No branches or pull requests

4 participants