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

Make LineLens.configChangedHandler update the config directly, instead of through a ref #1929

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Sep 13, 2023

WHAT

Not sure if this is the right thing to do, but:

I was looking at #1924 about the missing leading space in the lens text.
Whilst testing, I tried changing the user setting for the lens prefix from ' // ' to a custom value (e.g. ' ** ') and noticed that that wasn't picked up either.

It looks to me like the intent is for the 'default' value (the one changed in #1928) to be overwritten be a configuration specifed value that gets loaded in the configChangedHandler call made from the activate event.
However - when I tried debugging it it looked like the updated prefix value wasn't peristed into the config member of LineLens - it looked to always be using the local default.

I'm not sure what's causing that to go wrong, but - I'm not sure why the config object is passed into configChangedHandler by reference and from a bit of testing it seems that just making it update the config member directly causes the whole thing to correctly pick up a user specified prefix string.

So - here it is for any comments from anyone who knows more about how it's supposed to work.

WHY

refs #1924

HOW

Update the config member of LineLens directly instead of through a reference.

Copy link
Member

@TheAngryByrd TheAngryByrd left a comment

Choose a reason for hiding this comment

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

🎸 Thanks!!!

@TheAngryByrd TheAngryByrd marked this pull request as ready for review September 14, 2023 02:42
@TheAngryByrd TheAngryByrd merged commit c6aa663 into ionide:main Sep 14, 2023
1 of 2 checks passed
@Numpsy Numpsy deleted the rw/line_lens_tweak branch September 14, 2023 12:04
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.

3 participants