Skip to content

Conversation

AndreasArvidsson
Copy link
Member

No description provided.

@AndreasArvidsson AndreasArvidsson added this to the 0.24.0 milestone Dec 7, 2021
Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

Looks good with minor comment

Comment on lines +74 to +76
const decorationDebounceDelay = vscode.workspace
.getConfiguration("cursorless")
.get<number>("decorationDebounceDelay")!;
Copy link
Member

Choose a reason for hiding this comment

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

Do you think there's much of a performance impact to reading this one every time the document changes? The proper way to do this would be to set in constructor and then listen for settings changes, but I can't decide whether it's worth the effort. I'll leave it to you to decide whether to do that or just leave this one as is

Copy link
Member Author

Choose a reason for hiding this comment

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

Already tried I can't even measure the performance difference since the run to run variation is larger.
Looked at performance draw in task manager during full scroll up and down and uses 2-3% no matter if we read this setting or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Might actually be that vscode internally caches settings if the file hasn't updated

@AndreasArvidsson AndreasArvidsson merged commit 9ff51da into main Dec 7, 2021
@AndreasArvidsson AndreasArvidsson deleted the debounce_setting branch December 7, 2021 16:55
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