Skip to content

Support ISO 8601 in terminal link#278699

Merged
anthonykim1 merged 12 commits intomainfrom
anthonykim1/relativeTerminalLink1
Dec 1, 2025
Merged

Support ISO 8601 in terminal link#278699
anthonykim1 merged 12 commits intomainfrom
anthonykim1/relativeTerminalLink1

Conversation

@anthonykim1
Copy link
Contributor

@anthonykim1 anthonykim1 commented Nov 21, 2025

Resolves: #247568

@anthonykim1 anthonykim1 added this to the November 2025 milestone Nov 21, 2025
@anthonykim1 anthonykim1 requested a review from Tyriar November 21, 2025 03:54
@anthonykim1 anthonykim1 self-assigned this Nov 21, 2025
Copilot AI review requested due to automatic review settings November 21, 2025 03:54
@anthonykim1 anthonykim1 marked this pull request as ready for review November 21, 2025 03:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses issue #247568 by adding a negative lookbehind pattern to the terminal link parsing regex to prevent ISO 8601 timestamps from being incorrectly parsed as file paths with line/column numbers.

Key Changes

  • Added a negative lookbehind regex pattern (?<!\\d{4}-\\d{2}-\\d{2}T\\d{1,2}(?::\\d{1,2}){0,2}(?:[+-]\\d{2})?) to exclude ISO 8601 timestamps
  • Pattern prevents the link parser from matching timestamp components (like :45 in 10:45) as line numbers
  • Addresses reported issues with zero trimming and deletion of + characters in timestamps

Tyriar
Tyriar previously requested changes Nov 21, 2025
@Tyriar
Copy link
Member

Tyriar commented Nov 22, 2025

Looking a little closer so it's working find without the ./ as mentioned in the report. This is because the full link text is being included as a link candidate along side the ones with the badly detected line/col numbers. So first thing I would investigate is why the full file isn't being parsed out as a link candidate when the ./ prefix is included.

The fallback case where the quick pick shows up (search opener) isn't as important since if this is a file it should have been validated.

@anthonykim1 anthonykim1 marked this pull request as draft November 23, 2025 22:29
@anthonykim1
Copy link
Contributor Author

anthonykim1 commented Nov 24, 2025

Yeah, something is wrong with linkCandidates, even when link gets opened with search type. For both with and without ./ though.

Testing these: Screenshot 2025-11-21 at 4 33 07 PM

None of links in pics worked🥲, even without the ./ Pre-PR.

I thought the reason why the negative lookbehind or removing the pre-existing (?::|#| |['"],|, )${r()}([:.]${c()}(?:-(?:${re()}\\.)?${ce()})?)? + eolSuffix,` was because the ISO 8601 format got picked up with that pre-existing regex.

Lists in link candidates array seem odd because it would get picked up by (?::|#| |['"],|, )${r()}([:.]${c()}(?:-(?:${re()}\\.)?${ce()})?)? + eolSuffix,` without the negative lookbehind.

Debugging (pre-PR)

2025-11-21 20:51:15.806 [trace] [8efa26f] terminalLocalLinkDetector#detect linkCandidates [["/Users/anthonykim/Desktop/Skeleton/test-2025-04-28T11"]]

and

2025-11-23 21:52:07.343 [trace] [8efa26f] terminalLocalLinkDetector#detect linkCandidates [["/Users/anthonykim/Desktop/Skeleton/test-2025-04-28T11:03:09+02"]]

After this PR atm with negative lookbehind I still get:

2025-11-23 22:16:48.551 [trace] [8efa26f] terminalLocalLinkDetector#detect linkCandidates [["/Users/anthonykim/Desktop/Skeleton/test-2025-04-28T11"]]

but I'm able to open the links with search type:

2025-11-23 21:56:29.921 [debug] [8efa26f] Opening link [{"text":"./test-2025-04-28T11:03:09+02:00.log","bufferRange":{"start":{"x":1,"y":12},"end":{"x":36,"y":12}},"type":"Search","contextLine":"./test-2025-04-28T11:03:09+02:00.log"}]

Ideally I think they should be localFile like this one:

2025-11-23 21:57:05.454 [debug] [8efa26f] Opening link [{"text":"./testFile.py","uri":{"$mid":1,"path":"/Users/anthonykim/Desktop/Skeleton/testFile.py","scheme":"file"},"bufferRange":{"start":{"x":59,"y":13},"end":{"x":71,"y":13}},"type":"LocalFile","parsedLink":{"path":{"index":58,"text":"./testFile.py"}}}]

I should figure out why link candidate is wrong for both pre/post this this PR like suggested above, and maybe that would get us localFile link instead of search!

@anthonykim1 anthonykim1 force-pushed the anthonykim1/relativeTerminalLink1 branch from 1bb0508 to 15a9873 Compare November 24, 2025 17:16
@anthonykim1 anthonykim1 marked this pull request as ready for review December 1, 2025 05:48
@anthonykim1 anthonykim1 requested a review from Tyriar December 1, 2025 05:48
@anthonykim1 anthonykim1 enabled auto-merge (squash) December 1, 2025 05:49
@anthonykim1 anthonykim1 merged commit bc0c558 into main Dec 1, 2025
28 checks passed
@anthonykim1 anthonykim1 deleted the anthonykim1/relativeTerminalLink1 branch December 1, 2025 16:59
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jan 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terminal Ctrl+Click on a file with colon in filename does not open the file, preceding zeroes are deleted

4 participants