-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add support for Windows Terminal #8
Conversation
@jamestalmage could you please review&merge? |
@dropsonic I see the discussion here. I understand that a properly implemented terminal will just ignore the special characters, and show the link text. But that can tend to clobber output formatting and alignments. The goal of this module isn't simply to avoid printing control characters (as mentioned, a properly implemented terminal won't display those characters anyways), but to allow authors to conditionally format their output based on how it will be displayed. I think we maybe should do a version check. Is that possible? (If I'm understanding the conversations correctly, it seems no). I'm tempted to say get microsoft/terminal/issues/1040 sorted before we merge this. Otherwise we're going to end up clobbering a whole bunch of output. |
@jamestalmage that's correct, there is no way to determine the actual version of Windows Terminal. So, basically, it is a choice between My assumption is that the vast majority of users install updates from Microsoft Store on time (even automatically) so, in my opinion, (b) is the preferred option. |
I'm not a Windows user, so I've got limited insights into how frequently Windows Terminal is updated. I get your argument, and if Windows Terminal sees 99% auto-updates, then maybe it's a valid one. It would be good to understand that as well. In the most common use case for this package, there's a big difference between returning a false negative, and a false positive. I think this use from eslint-formatter-pretty is fairly representative. If we were to give a false negative, you are inconvenienced and have to google the corresponding eslint rule to find the documentation. If we give a false positive, the output is clobbered and almost unusable. So, I'm always going to be pretty conservative and favor false negatives over false positives. The bar has to be higher than just "It should be OK for most users", we need to ensure it is right for very nearly everybody. Before this package, tool authors were avoiding terminal hyperlinks entirely to maintain the widest possible compatibility. This package encourages more hyperlink use by guaranteeing dependents their output will be usable in the widest array of circumstances. If somebody from Microsoft is willing to provide some insight into the install base of non-supporting vs supporting terminals, I am willing to reconsider. But it seems like it would be a lot more straightforward for them to just implement any of the recommended solutions in microsoft/terminal/issues/1040. This won't be the last time someone wants to interrogate the terminal version. |
@jamestalmage makes total sense, thanks for the detailed explanation. Ok, let's wait. |
FYI: They are officially recommend installing the Windows Terminal from Microsoft Store: https://github.com/microsoft/terminal#microsoft-store-recommended. They also provided some ways to install Windows Terminal with package managers and easy to update/upgrade: https://github.com/microsoft/terminal#other-install-methods. |
Can this be merged now without the need for a version check? Windows terminal has supported links for almost 4 years now. Similar libraries in other languages include it. |
Closes #7