microsoft / codetour

VS Code extension that allows you to record and play back guided tours of codebases, directly within the editor.
https://aka.ms/codetour
MIT License
4.36k stars 113 forks source link

Fix invalid tour references #96

Closed TheSench closed 3 years ago

TheSench commented 3 years ago

Overview

This PR addresses (at least in part) the issue of the TOUR_REFERENCE_PATTERN regex identifying things like empty square brackets identified in #81. It is not a full fix, as code-fences and inline code-statements are still processed, but it reduces the number of false-positives found by the regex.

Description

The regex used to find tour references had the linkTitle, tourTitle, and stepNumber all as optional. The replacement logic assumes that if tourTitle is missing, then at least stepNumber is present. I updated the regex to ensure that the block including tourTitle/stepNumber is at least non-empty (and not just whitespace).

Testing

I used the following script to test various forms of valid and invalid links:

Here's a script:

```javascript
var test = [];
String[] arr;

[Link][SomeTour#123]

[]

[linkTitle][]

[][tourTitle]

[][#1]

[linkTitle][#2]

[][tourTitle#3]


| **Before**| **After**|
|-----|-------|
| ![image](https://user-images.githubusercontent.com/9260413/97793722-2e592200-1bc6-11eb-9923-7b403a854019.png) | ![image](https://user-images.githubusercontent.com/9260413/97792705-f814a600-1bb7-11eb-8273-cf452aa1b703.png) |
lostintangent commented 3 years ago

This is awesome, thanks! Looking at your test case results: it would be great if we could also support “[Link][SomeTour#123]”, since that allows you to reference a specific step in another tour.

TheSench commented 3 years ago

This is awesome, thanks! Looking at your test case results: it would be great if we could also support “[Link][SomeTour#123]”, since that allows you to reference a specific step in another tour.

I can take a look at that sometime later this weekend.

On an unrelated note, it looks like my editor changed some unrelated formatting on accident, so I'll also get that reverted.

lostintangent commented 3 years ago

@TheSench Sounds great. Thanks!

TheSench commented 3 years ago

This is awesome, thanks! Looking at your test case results: it would be great if we could also support “[Link][SomeTour#123]”, since that allows you to reference a specific step in another tour.

Turns out I wasn't looking closely enough at my test case - I hadn't defined the tours being referenced by title. That support is still there, I just needed the proper data set up. I've updated my before/after screenshots and rebased the formatting changes out of the PR.

lostintangent commented 3 years ago

OK awesome. Thanks for this contribution!