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

Gerrit: parse change-ids in the whole message body, not only in the footer #2251

Open
felipecrs opened this issue Oct 7, 2022 · 5 comments · May be fixed by #2255
Open

Gerrit: parse change-ids in the whole message body, not only in the footer #2251

felipecrs opened this issue Oct 7, 2022 · 5 comments · May be fixed by #2255
Labels
feature New feature or request

Comments

@felipecrs
Copy link
Contributor

felipecrs commented Oct 7, 2022

In Gerrit commit messages, you can include as many change ids as you want. Gerrit's UI will add a hyperlink to each change id found. See:

image

However, Gitlens currently only parses the last one, in the footer:

image

PS: I'm working on a PR. Edit: not anymore.

@felipecrs felipecrs added feature New feature or request triage Needs to be looked at labels Oct 7, 2022
@eamodio
Copy link
Member

eamodio commented Oct 7, 2022

TY

@eamodio eamodio removed the triage Needs to be looked at label Oct 7, 2022
@felipecrs
Copy link
Contributor Author

felipecrs commented Oct 9, 2022

Sorry, @eamodio, I wrongly assumed that Gitlens supported some kind of regex-based autolinks reference, but unfortunately it does not as of now.

It has been previously suggested/reported:

So, I can say that this depends on that to be implemented first.


This is to say: please don't expect a PR from me in the short-term anymore. If anyone else wants to work on this issue (or the referenced issues), feel totally free.

@felipecrs felipecrs linked a pull request Oct 10, 2022 that will close this issue
9 tasks
@felipecrs
Copy link
Contributor Author

@eamodio actually here is the PR: #2255, I just need a little bit of help to finish it.

@eamodio
Copy link
Member

eamodio commented Oct 12, 2022

@felipecrs thanks -- I haven't had a chance to look at that PR yet as we are focused on getting GitLens 13 out early next week. But I did want to mention that built-in remote providers can provide their own tokenization/parsing for autolinks, see

override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] {
if (this._autolinks === undefined) {
this._autolinks = [
{
prefix: '#',
url: `${this.baseUrl}/issues/<num>`,
title: `Open Issue or Pull Request #<num> on ${this.name}`,
description: `Issue or Pull Request #<num> on ${this.name}`,
},
{
prefix: 'gh-',
url: `${this.baseUrl}/issues/<num>`,
title: `Open Issue or Pull Request #<num> on ${this.name}`,
ignoreCase: true,
description: `Issue or Pull Request #<num> on ${this.name}`,
},
{
tokenize: (
text: string,
outputFormat: 'html' | 'markdown' | 'plaintext',
tokenMapping: Map<string, string>,
) => {
return outputFormat === 'plaintext'
? text
: text.replace(autolinkFullIssuesRegex, (linkText: string, repo: string, num: string) => {
const url = encodeUrl(`${this.protocol}://${this.domain}/${repo}/issues/${num}`);
const title = ` "Open Issue or Pull Request #${num} from ${repo} on ${this.name}"`;
const token = `\x00${tokenMapping.size}\x00`;
if (outputFormat === 'markdown') {
tokenMapping.set(token, `[${linkText}](${url}${title})`);
} else if (outputFormat === 'html') {
tokenMapping.set(token, `<a href="${url}" title=${title}>${linkText}</a>`);
}
return token;
});
},
parse: (text: string, autolinks: Map<string, Autolink>) => {
let repo: string;
let num: string;
let match;
do {
match = autolinkFullIssuesRegex.exec(text);
if (match?.groups == null) break;
({ repo, num } = match.groups);
autolinks.set(num, {
provider: this,
id: num,
prefix: `${repo}#`,
url: `${this.protocol}://${this.domain}/${repo}/issues/${num}`,
title: `Open Issue or Pull Request #<num> from ${repo} on ${this.name}`,
description: `Issue or Pull Request #${num} from ${repo} on ${this.name}`,
});
} while (true);
},
},
];
}
return this._autolinks;
}

@felipecrs
Copy link
Contributor Author

felipecrs commented Oct 20, 2022

@eamodio Thanks for the heads up. This is really cool and I could definitely make use of it to fix the Gerrit specific issue.

However, it would be even nicer if we could get the Support user-defined regex for auto-links as we could solve many issues at once, including this one for Gerrit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants