"Open link" stopped working

[rafeca]

Description

"Open in link" has stopped working on the Electron v3 branch (source).

Steps to Reproduce

1. Download an Atom v.138.0-beta0.
2. Open a file that contains a url.
3. Right click on the url and click on "Open link"

Expected behavior: The URL should be opened in the default web browser.

Actual behavior: Nothing happens

Reproduces how often: 100% of times

Versions

Atom v1.38.0-beta0
Atom v1.39 nightlies

Additional Information

To open external links we use the electron shell.openExternal() API, but in other places inside Atom links seem to get opened correctly (for example the ones on the welcome package), so it may be something to do with the handling logic for the link:open action (code pointer).

[rafeca]

This seems to only be happening on JSON files, and is caused by the extracted token from the editor (code pointer) containing quotes.

Based on that information, my hypothesis is that this is caused by atom/language-json#68, but that wouldn't explain why it's only happening on Electron v3...

[rafeca]

Oops, I've just download the latest Atom nightly which runs on electron v2 and this issue is present there as well, so indeed that's a problem in the treesitter grammar of language-json and not related to electron.

[50Wliu]

@Ben3eeE I believe you were close to a resolution with this, right?

[Arcanemagus]

The solution depended on #19336 as far as I understand. Now that that is in place we just need to fix it in language-json (and the other languages that distinguish URLs from what I understand).

[jasonrudolph]

@Ben3eeE: I think this is related to the changes in atom/language-json#68 and atom/language-json#73, so I want to check in with you to see if you have thoughts on the preferred way to resolve this issue.

Findings

Here's what I'm seeing so far:

When viewing a JSON file in Atom 1.37.0, when the cursor is positioned inside a double-quoted string containing a URL, the scopes appear as follows:

In Atom 1.39.0-nightly9, the scopes appear as follows:

Because of that difference, the link package gets different results when it tries to identify the URL in the JSON file. When running the Link: Open command in Atom 1.37.0, the resulting token contains only the URL string:

When running the Link: Open command in Atom 1.39.0-nightly9, the resulting token contains a URL string wrapped in double quotes:

When Atom 1.39.0-nightly9 tries to use the double-quoted string as a URL, it fails.

Potential resolutions

I see a couple potential resolutions:

• We could change language-json so that it generates scopes in the same way it did in previous versions. Specifically, instead of generating a string.quoted.double.markup.underlink.link scope for the double-quoted URL string, it would generate a string.quoted.double.json scope for the double-quoted string, and a markup-underlink.link.https.hyperlink scope for the URL itself (without the surrounding double quotes).
• Alternatively, we could probably change the link package to strip the surrounding double quotes when it encounters a string.quoted.double.markup.underlink.link scope.

@Ben3eeE: Given your familiarity with the recent enhancements to language-json, do you have a preference for which approach I take? Or, if there's another approach that would work better, just let me know. 🙇

[jasonrudolph]

A quick update:

@Ben3eeE and I discussed the potential resolutions described in #19353 (comment):

• We could change language-json so that it generates scopes in the same way it did in previous versions. Specifically, instead of generating a string.quoted.double.markup.underlink.link scope for the double-quoted URL string, it would generate a string.quoted.double.json scope for the double-quoted string, and a markup-underlink.link.https.hyperlink scope for the URL itself (without the surrounding double quotes).
• Alternatively, we could probably change the link package to strip the surrounding double quotes when it encounters a string.quoted.double.markup.underlink.link scope.

The first option benefits all consumers of language-json, so that feels like a more desirable resolution, whereas the second option is more of a patch job. 😇 With that in mind, I'm heading in the general direction of the first option above.

To make these changes to language-json, we first need to enhance tree-sitter-json to provide a syntax node for the internal content of JSON strings. (For example, given the string "abc", we need tree-sitter-json to provide a syntax node for abc.) I think tree-sitter/tree-sitter-json@7252a04 will meet this need. Before opening a pull request in tree-sitter-json with that change, I'll first try to get language-json using that change locally, and then see if I can use my local version of language-json to resolve this issue. If all of that works out, then I'll open pull requests for each of these changes. Stay tuned. 😄📺

[jasonrudolph]

Before opening a pull request in tree-sitter-json with that change, I'll first try to get language-json using that change locally, and then see if I can use my local version of language-json to resolve this issue. If all of that works out, then I'll open pull requests for each of these changes.

I've got things working locally now. I've pushed up the changes in atom/language-json@6595ca7. That commit currently depends on my fork of tree-sitter-json, so my next step is to open a pull request in tree-sitter-json to introduce the changes in tree-sitter/tree-sitter-json@7252a04. Once that's done, I'll open a pull request for the language-json changes and ask for review.

[lock]

This issue has been automatically locked since there has not been any recent activity after it was closed. If you can still reproduce this issue in Safe Mode then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue. Thanks!