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

ref #648 provide completion short hand and long hand notation with similar scope of legacy #668

Conversation

apupier
Copy link
Member

@apupier apupier commented Nov 18, 2022

Provide completion after xref for ids defined with shorthand and
longhand notation

I.e. with [#myId] and [id=myId]

part of #648

image

based on #667

const regex = /\[id=(\w+)\]/g
const matched = content.match(regex)
if (matched) {
return matched.map((result) => result.replace('[id=', '').replace(']', ''))
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to use the (named) group? Since we are using (\w+) we can use the group reference $1

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

matchAll is available on Node 12. Are you sure it does not work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have this error:
image

Copy link
Member

Choose a reason for hiding this comment

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

You can safely update the target to es2020.

I'm planning to make the switch: #506

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer that we keep it in a separate Pull request. I'm not even sure how to change the target correctly neither what are the implications

Copy link
Member

Choose a reason for hiding this comment

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

All good 👍🏻

@ggrossetie
Copy link
Member

ggrossetie commented Nov 21, 2022

Thanks for your work! I found a few issues with the existing code so feel free to fix them in this pull request or we can create an issue and do a follow-up pull request.

@apupier apupier force-pushed the 648-provideCompletionSHortHandNadLongHandNotationWithSimilarScopeOfLegacy branch from 62bd83e to 0fea3ca Compare November 25, 2022 13:22
longhand notation

I.e. with [#myId] and [id=myId]

part of asciidoctor#648

Signed-off-by: Aurélien Pupier <[email protected]>
@apupier apupier force-pushed the 648-provideCompletionSHortHandNadLongHandNotationWithSimilarScopeOfLegacy branch from 0fea3ca to 7ed4886 Compare November 26, 2022 16:35
@apupier apupier requested a review from ggrossetie November 26, 2022 16:38
@ggrossetie ggrossetie changed the title 648 provide completion short hand nad long hand notation with similar scope of legacy ref #648 provide completion short hand and long hand notation with similar scope of legacy Dec 23, 2022
@ggrossetie ggrossetie merged commit 823bb22 into asciidoctor:master Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants