SupportsHyperlinks() returning true on macOS default Terminal, when they do not work as expected

[clicktravel-james]

Hi authors 👋

Happy Holidays 🎄

Issue Summary

We are developing a CLI tool and wanted to embed some links using this library. We found that the support for links is somewhat varied across the different terminal types supported by macOS. This is fine but we have had to add our own logic to work out which of these Terminal applications support links.

This is what we expected the feature SupportsHyperlinks() todo for us. Sadly on the default Terminal application that is provided with the macOS, the return value for SupportsHyperlinks() is true but the link is hidden and not clickable. The text appears in the console like all other text and you would not know it was special to any other fmt.Println() statement.

Change request

Is it possible to improve the library so that it is aware of this feature being disable in the macOS Terminal app so we can remove our simple bespoke code that is doing this logic for us?

Versions

GO Version: go version go1.21.6 darwin/arm64
Library Version: github.com/savioxavier/termlink v1.4.1
macOS version: Sonoma 14.7.1 (23H222)
Terminal version: Version 2.14 (453)

Screenshots

macOS Terminal application

iterm2 terminal application

VS Code

Example simple bespoke patch implementation in our code

func isUsingATerminalWhichSupportsLinks() bool {
	termProgram := os.Getenv("TERM_PROGRAM")
	switch termProgram {
	case "iTerm.app":
		return true
	case "vscode":
		return true
	default:
		return false
	}
}

[savioxavier]

Hey, first of all, thank you very much for reporting this issue! I don't use macOS, so I didn't really know this was a thing.

The code that checks the hyperlink support for terminals that have the TERM_PROGRAM environment variable is as follows:

termlink/termlink.go

Lines 105 to 125 in 981ec65

	if hasEnv("TERM_PROGRAM") {
	v := parseVersion(getEnv("TERM_PROGRAM_VERSION"))
	switch term := getEnv("TERM_PROGRAM"); term {
	case "iTerm.app":
	if v.major == 3 {
	return v.minor >= 1
	}
	return v.major > 3
	case "WezTerm":
	// Even though WezTerm's version is something like 20200620-160318-e00b076c
	// parseVersion will still parse it with a standalone major segment (ie: 20200620)
	// with minor and patch segments set to 0
	return v.major >= 20200620
	case "vscode":
	return v.major > 1 || (v.major == 1 && v.minor >= 72)
	// Hyper Terminal used to be included in this list, and it even supports hyperlinks
	// but the hyperlinks are pseudo-hyperlinks and are actually not clickable
	}
	}

I suppose one of the reasons why this happens is cause I forgot to add a default: return false to the switch statement. The reason why the true is returned for Apple Terminal is probably (though I'm not sure) cause the TERM variable is set to "xterm-256color". I'm not really sure why I added that, now that I think of it.

I'll implement a fix in a separate branch and make the following changes:

1. Add a default case to the switch statement
2. Remove "xterm-256color" from the list of TERM variables

[savioxavier]

Hi, I just released a change in branch apple-terminal-fix. Could you please pull the branch and run a test (maybe with the example file) to see if the issue has been fixed? Just wanna confirm everything's all right before I release it, thanks!

[clicktravel-james]

Hi,

Happy new Year and thank you for responding to me so quickly. My apologises for taking a while to get back to you, I have been AFK for the Christmas holidays.

So I have pulled your new branch apple-terminal-fix and have run a few tests locally on my mac. Specifically I have run the example command go run examples/start.go.

The screenshot below is what I now see on the main mac terminal app when I run that command. This seems to be the expected behaviour and is different to what I see if I run it on the main branch, which clearly shows the old behaviour.

I have also tested it on ITerm2 and VSCode for mac and both still work as they did before.

With that said I think that your new fix is delivering the requested change, which is amazing and should allow us to remove our bespoke code once merged 👍

[savioxavier]

Thank you for your contribution

Fixed in https://github.com/savioxavier/termlink/releases/tag/v1.4.2