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

Feat: follow link prefixed with file:// in the string content using documentLinkProvider #27

Conversation

WilsonZiweiWang
Copy link
Contributor

Use the documentLinkProvider to provide the correct links in the string content.

@WilsonZiweiWang WilsonZiweiWang added the help wanted Extra attention is needed label Dec 6, 2023
@WilsonZiweiWang WilsonZiweiWang self-assigned this Dec 6, 2023
@WilsonZiweiWang
Copy link
Contributor Author

WilsonZiweiWang commented Dec 6, 2023

@deribaucourt
The workflow to provide the document links should be fine in this PR. However, I was encountering some difficulties in finding the files using the vscode.workspace.findFiles. Had no luck in debugging it.

@WilsonZiweiWang WilsonZiweiWang changed the title Feat: follow link prefixed with file:// in the string content Feat: follow link prefixed with file:// in the string content using documentLinkProvider Dec 6, 2023
Copy link
Member

@deribaucourt deribaucourt left a comment

Choose a reason for hiding this comment

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

I pushed a new commit that fixes vscode.search

server/src/tree-sitter/analyzer.ts Outdated Show resolved Hide resolved
link.range.end.character
)
try {
const [file] = await vscode.workspace.findFiles(link.value, undefined, 1)
Copy link
Member

Choose a reason for hiding this comment

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

This gets called non stop and saturates all my CPUs to 100% for too long untill all links are generated. Make sure it's only called only when a "cltr+click" event is generated or the least amount of time possible. (opening busybox_*.bb)

Copy link
Member

Choose a reason for hiding this comment

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

Also after I click a first time, it takes forever for the links to work again.

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe it's just a performance issue in the for loops.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your fix is very good.

Copy link
Member

Choose a reason for hiding this comment

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

There still was a performance issue in your latest branch. I pushed a commit that greatly reduces the CPU consumption when opening recipes with many defined file:// links.

client/src/documentLinkProvider.ts Outdated Show resolved Hide resolved
@deribaucourt deribaucourt removed the help wanted Extra attention is needed label Dec 7, 2023
@WilsonZiweiWang WilsonZiweiWang force-pushed the Feature-10430-go-to-definition-for-SRC-URI-documentLinks branch from b63bf68 to 88f077a Compare December 7, 2023 16:52
Copy link
Member

@deribaucourt deribaucourt left a comment

Choose a reason for hiding this comment

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

LGTM, I pushed a commit for performance

@deribaucourt deribaucourt force-pushed the Feature-10430-go-to-definition-for-SRC-URI-documentLinks branch from 82346e0 to 670bac7 Compare December 8, 2023 15:41
@deribaucourt deribaucourt merged commit 2364e83 into yoctoproject:staging Dec 11, 2023
1 check passed
@deribaucourt deribaucourt deleted the Feature-10430-go-to-definition-for-SRC-URI-documentLinks branch December 11, 2023 08:23
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