search

spectreconsole / spectre.console

A .NET library that makes it easier to create beautiful console applications.
https://spectreconsole.net
MIT License
9.45k stars 499 forks source link

Ansi Links capability detection is not very precise #764

Open rouke-broersma opened 2 years ago

rouke-broersma commented 2 years ago

Is your feature request related to a problem? Please describe. Currently as far as I could determine Ansi link support is determined by checking if a terminal supports Ansi, and the terminal is not cmd.exe (basically what it comes down to I think) or has link support disabled explicitly through an Enhancer. There are as far as I can tell a lot of terminal emulators that support for example Ansi colours but not Ansi links (powershell, visual studio developer terminal, visual studio debugger console). I believe this combination can mean that if you use the capability check to decide if you can pretty-print a uri on a given terminal that you can end up printing something entirely unusable to the user, because the terminal cannot interpret the uri portion of the link and only prints the text portion. To be safe I think I should choosing to always print the full uri as the link text, which kinda defeats the purpose of using Ansi links to pretty print in the first place.

Describe the solution you'd like The capability detection should determine whether or not the terminal in question actually supports Ansi links, not just that it supports Ansi.

Additional context I know a library for the npm ecosystem that tries to do this, it involves sometimes specific checks for specific terminals or environments. This might be too much of a maintenance headache but at least they provide a starting point/shared codebase. See: https://github.com/jamestalmage/supports-hyperlinks

Please upvote :+1: this issue if you are interested in it.

rouke-broersma commented 2 years ago

I want to add that this is not at all a pressing issue, just something I noticed on a PR by a contributor who introduced spectre.console for this specific purpose. For now we will simply print the filepath as the link text.

patriksvensson commented 2 years ago

There currently is no standardized way of detecting this capability, so we just make assumptions. We're happy to update the rules for this if you have any concrete suggestions.

rouke-broersma commented 2 years ago

I notice that modern terminal emulators that I've tried for the most part support this capability so since there is no way to detect support I think the easiest way to deal with this would be to blacklist known bad cases such as powershell, visual studio developer terminal, visual studio debugger console, cmd.exe. If it is possible to detect those environments of course. I think CI environments can also be blacklisted in general (I think at the moment only one CI is explicitly set to links not supported) because you usually access those reports from the browser, where it is way more likely for the browser to understand uri syntax than ANSI magic.

On top of that, but perhaps Spectre.Console already does this but, my preferred way to use this would actually be that I don't have to check for the capability at all, but that the link markup renderer does the detection and in the case of no ansi link support that it prints the uri instead of the link text (since some terminals already support uri's but not ansi links).

phil-scott-78 commented 2 years ago

Can you send a reproduction of what you are seeing that's invalid? We are just sending ANSI sequences for the link markup which should be ignored by the console if they don't support it. E.g. here's the regular conhost running the links example. Just doesn't display the link. Are you seeing the ANSI sequences or is the output corrupt?

image

rouke-broersma commented 2 years ago

Hi @phil-scott-78 ,

Not displaying the link is the problem. Now a user of the app just sees 'click here' without being able to click on the link which means in our case that the user will not know where their html report with the execution result is located.

If you look at the source of the example you'll see that it's supposed to print something like links are not supported if links are not supported. It does not.

patriksvensson commented 2 years ago

@rouke-broersma You can always set the Console.Profile.Capabilities.Links (see https://github.com/spectreconsole/spectre.console/blob/main/src/Spectre.Console/Capabilities.cs#L25) to false, or create a special profile enricher if you know that the environment won't be able to display the link correctly.

console.Profile.Capabilities.Links = false;
rouke-broersma commented 2 years ago

@patriksvensson until there is a way to detect ansi link support we have the workaround of printing the link path as the link text. This is what we had before anyway so it is acceptable. It would be nice if some day there would be a way to detect this so we can make it prettier.

Would you know of a place where I could suggest something like this? So far it seems like the ansi link feature comes entirely from one user with a gist on GitHub who's been pushing for support at well known terminal maintainers..