Skip to content

Fix auto provider detection#10772

Merged
Colengms merged 6 commits intomainfrom
coleng/fix_stuck_provider_id
Apr 5, 2023
Merged

Fix auto provider detection#10772
Colengms merged 6 commits intomainfrom
coleng/fix_stuck_provider_id

Conversation

@Colengms
Copy link
Copy Markdown
Contributor

@Colengms Colengms commented Apr 5, 2023

We're seeing a scenario in which one provider will register, get automatically selected because it's the only one (and then provide a browse configuration), but not get properly un-selected when a subsequent provider registers.

Removed a check that was opportunistically using a configuration provider if registered and if we received a browse configuration for it, as this scenario would lead to the 'too early' automatically selected config provider being kept.

We have a pattern in place to clear out any modifications we had made to the configuration (by calling parsePropertiesFile again) when we need to reprocess it. But, that was a NO-OP if there was no c_cpp_properties.json file, so our modifications would not get removed in that scenario (making the prior registered configuration provider look like the user had explicitly specified it).  I updated that to instead use the default configuration, and I needed to fix one place where we didn't do that reset if the contents were empty.

Copy link
Copy Markdown
Contributor

@sean-mcmanus sean-mcmanus left a comment

Choose a reason for hiding this comment

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

This breaks the 1 provider auto-select scenario (the 2 provider scenario is fixed). I see this.configuration.CurrentConfigurationProvider return '' instead of the 1 provider.

@Colengms Colengms merged commit 605c803 into main Apr 5, 2023
@Colengms Colengms deleted the coleng/fix_stuck_provider_id branch April 5, 2023 23:02
@github-actions github-actions Bot locked and limited conversation to collaborators May 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants