Skip to content
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

Show hover cards when issue numbers are followed by , or . #2085

Closed
StephanTLavavej opened this issue Aug 23, 2020 · 0 comments · Fixed by #2106
Closed

Show hover cards when issue numbers are followed by , or . #2085

StephanTLavavej opened this issue Aug 23, 2020 · 0 comments · Fixed by #2106
Assignees
Labels
Milestone

Comments

@StephanTLavavej
Copy link
Member

This issue is similar to #1813, but not a duplicate.

  • Extension version: 0.19.0
  • VSCode Version: 1.48.1
  • OS: Windows_NT x64 10.0.19041

Steps to Reproduce:

  1. Hover over // GH-2085, blah blah.
  2. Result: no hover card 😿
  3. Hover over // Test GH-2085.
  4. Result: no hover card 😿

hover2

The regex mentioned in #1813 currently considers ($|[\s\:\;\-\(\=\)]) after an issue number:

export const ISSUE_EXPRESSION = /(([^\s]+)\/([^\s]+))?(#|GH-)([1-9][0-9]*)($|[\s\:\;\-\(\=\)])/;
export const ISSUE_OR_URL_EXPRESSION = /(https?:\/\/github\.com\/(([^\s]+)\/([^\s]+))\/([^\s]+\/)?(issues|pull)\/([0-9]+)(#issuecomment\-([0-9]+))?)|(([^\s]+)\/([^\s]+))?(#|GH-)([1-9][0-9]*)($|[\s\:\;\-\(\=\)])/;

Extending this to include , and . would be nice, but (as mentioned in #1813 (comment) ) it really seems like this should simply be looking for a word boundary with \b. That would allow hover cards to activate for comments like // Is this GH-2085?, // Avoid the problem mentioned in GH-2085!, /***Test GH-2085***/, etc.


As an aside, most characters in character classes lose their specialness and don't need to be escaped; r1 and r2 behave identically:

const r1 = /[\s\:\;\-\(\=\)]/;
const r2 = /[\s:;\-(=)]/;
for (const c of [' ', ':', ';', '-', '(', '=', ')', 'x']) {
  console.log(`'${c}': ${r1.test(c) === r2.test(c)}`);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants