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
Adds support for regex in autolinks #2255
base: main
Are you sure you want to change the base?
Conversation
"required": [ | ||
"prefix", | ||
"url" | ||
"oneOf": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This produces:
Code_McgfIrbujZ.mp4
Update: we can probably use regexp-tree to parse the regex and escape it properly for markdown and html. |
9923d02
to
9a1266a
Compare
…to parse-gerrit-change-id
Ok, this is now working. The implementation may be a little rough though. You can test with the following repo: git clone https://review.gerrithub.io/felipecrs/gerrit-jenkins And the last commit. It comes with a settings.json like:
Demo: Code_3ute0TidEL.mp4 |
This would be awesome and open up matching on JIRA ticket ids in branch names, ex: |
@eamodio, any chance to get that merged? |
@@ -440,24 +448,42 @@ function ensureCachedRegex( | |||
): asserts ref is RequireSome<CacheableAutolinkReference, 'messageRegex'>; | |||
function ensureCachedRegex(ref: CacheableAutolinkReference, outputFormat: 'html' | 'markdown' | 'plaintext') { | |||
// Regexes matches the ref prefix followed by a token (e.g. #1234) | |||
function escapeRegexCharacters(regex: string, fn: (c: string) => string) { | |||
return regexpTree.transform(`/${ regex }/`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it would be too hard to achieve the same result without having to add a new dependency to vscode-gitlens 🤔
regexp-tree is really cool, nonetheless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried very hard and could not find any other way sadly.
Actually it took me a long way to reach this state even.
lol
Could we clarify what changes must be made to this implementation in order to get it merged (other than resolving conflicts)? Is the added dependency a dealbreaker, or could it be acceptable as an interim solution? Would love to see this functionality hit main! |
Description
Adds support for autolink references based on regular expression for more advanced use cases, while keeping backwards compatibily with the existing prefix-based cases.
Additionally, it refactors the Gerrit autolinks to leverage the new option:
Checklist
Fixes $XXX -
orCloses #XXX -
prefix to auto-close the issue that your PR addressesExtras: