Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit970bf1e

Browse files
majochaTIHan
authored andcommitted
Visual Studio Editor Options bug fixes (dotnet#5999)
* fix options ui logic and syncing between IDE instances* add comments* feedback
1 parentefb68b8 commit970bf1e

File tree

3 files changed

+92
-39
lines changed

3 files changed

+92
-39
lines changed

‎vsintegration/src/FSharp.Editor/Options/EditorOptions.fs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,19 +119,19 @@ type EditorOptions
119119
store.Register CodeLensOptions.Default
120120
store.Register FormattingOptions.Default
121121

122-
member__.IntelliSense:IntelliSenseOptions= store.Read()
123-
member__.QuickInfo:QuickInfoOptions= store.Read()
124-
member__.CodeFixes:CodeFixesOptions= store.Read()
125-
member__.LanguageServicePerformance:LanguageServicePerformanceOptions= store.Read()
126-
member__.Advanced:AdvancedOptions= store.Read()
127-
member__.CodeLens:CodeLensOptions= store.Read()
128-
member__.Formatting:FormattingOptions= store.Read()
122+
member__.IntelliSense:IntelliSenseOptions= store.Get()
123+
member__.QuickInfo:QuickInfoOptions= store.Get()
124+
member__.CodeFixes:CodeFixesOptions= store.Get()
125+
member__.LanguageServicePerformance:LanguageServicePerformanceOptions= store.Get()
126+
member__.Advanced:AdvancedOptions= store.Get()
127+
member__.CodeLens:CodeLensOptions= store.Get()
128+
member__.Formatting:FormattingOptions= store.Get()
129129

130130
interface Microsoft.CodeAnalysis.Host.IWorkspaceService
131131

132132
interface IPersistSettingswith
133-
member__.Read()= store.Read()
134-
member__.Write(settings)= store.Write(settings)
133+
member__.LoadSettings()= store.LoadSettings()
134+
member__.SaveSettings(settings)= store.SaveSettings(settings)
135135

136136

137137
[<AutoOpen>]

‎vsintegration/src/FSharp.Editor/Options/SettingsPersistence.fs‎

Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,51 +9,78 @@ open Microsoft.VisualStudio.Settings
99
openNewtonsoft.Json
1010

1111
typeIPersistSettings=
12-
abstractmemberRead :unit->'t
13-
abstractmemberWrite :'t->unit
12+
abstractmemberLoadSettings :unit->'t
13+
abstractmemberSaveSettings :'t->unit
1414

1515
[<Guid(Guids.svsSettingsPersistenceManagerIdString)>]
1616
typeSVsSettingsPersistenceManager=classend
1717

1818
typeSettingsStore(serviceProvider: IServiceProvider)=
1919

2020
letsettingsManager= serviceProvider.GetService(typeof<SVsSettingsPersistenceManager>):?> ISettingsManager
21-
// settings quallified type names are used as keys, this should be enough to avoid collisions
22-
letstorageKey(typ:Type)= typ.Namespace+"."+ typ.Name
21+
22+
letstorageKeyVersions(typ:Type)=
23+
// "TextEditor" prefix seems to be required for settings changes to be synced between IDE instances
24+
["TextEditor.FSharp."+ typ.Namespace+"."+ typ.Name
25+
// we keep this old storage key to upgrade without reverting user changes
26+
typ.Namespace+"."+ typ.Name]
27+
28+
letstorageKey(typ:Type)= storageKeyVersions typ|> List.head
2329

2430
// Each group of settings is a value of some named type, for example 'IntelliSenseOptions', 'QuickInfoOptions'
25-
// We cache exactly one instance of each, treating them as immutable.
26-
// This cache is updated by the SettingsStore when the user changes an option.
27-
letcache= System.Collections.Concurrent.ConcurrentDictionary<Type, obj>()
31+
// and it is usually representing one separate option page in the UI.
32+
// We cache exactly one immutable value of each type.
33+
// This cache is updated by the SettingsStore when the user makes changes in the Options dialog
34+
// or when a change is propagated from another VS IDE instance by SVsSettingsPersistenceManager.
35+
letcache= ConcurrentDictionary<Type, obj>()
2836

29-
letread()=
37+
letgetCached()=
3038
match cache.TryGetValue(typeof<'t>)with
31-
|true,value->value:?> 't
32-
|_-> failwithf"Settings%s are not registered." typeof<'t>.Name
39+
|true,(:? 'tasvalue)-> value
40+
|_-> failwithf"Settings%s are not registered." typeof<'t>.Name
3341

34-
letwrite settings= cache.[settings.GetType()]<- settings
42+
letkeepInCache settings= cache.[settings.GetType()]<- settings
43+
44+
// The settings record, even though immutable, is being effectively mutated in two instances:
45+
// when it is passed to the UI (provided it is marked with CLIMutable attribute);
46+
// when it is being populated from JSON using JsonConvert.PopulateObject;
47+
// We make a deep copy in these instances to isolate and contain the mutation
48+
letclone(v:'t)= JsonConvert.SerializeObject v|> JsonConvert.DeserializeObject<'t>
3549

3650
letupdateFromStore settings=
37-
letresult,json= settings.GetType()|> storageKey|> settingsManager.TryGetValue
38-
if result= GetValueResult.Successthen
39-
// if it fails we just return what we got
40-
try JsonConvert.PopulateObject(json, settings)with_->()
41-
settings
42-
43-
member__.Read()= read()
51+
// make a deep copy so that PopulateObject does not alter the original
52+
letcopy= clone settings
53+
// if the new key is not found by ISettingsManager, we try the old keys
54+
// so that user settings are not lost
55+
settings.GetType()|> storageKeyVersions
56+
|> Seq.map(settingsManager.TryGetValue)
57+
|> Seq.tryPick(function GetValueResult.Success, json-> Some json|_-> None)
58+
|> Option.iter(fun json->try JsonConvert.PopulateObject(json, copy)with_->())
59+
copy
4460

45-
member__.Write settings=
46-
write settings
47-
// we replace default serialization with Newtonsoft.Json for easy schema evolution
61+
member__.Get()= getCached()
62+
63+
// Used by the AbstractOptionPage to populate dialog controls.
64+
// We always have the latest value in the cache so we just return
65+
// cloned value here because it may be altered by the UI if declared with [<CLIMutable>]
66+
member__.LoadSettings()= getCached()|> clone
67+
68+
member__.SaveSettings settings=
69+
// We replace default serialization with Newtonsoft.Json for easy schema evolution.
70+
// For example, if we add a new bool field to the record, representing another checkbox in Options dialog
71+
// deserialization will still work fine. When we pass default value to JsonConvert.PopulateObject it will
72+
// fill just the known fields.
4873
settingsManager.SetValueAsync(settings.GetType()|> storageKey, JsonConvert.SerializeObject settings,false)
49-
|> Async.AwaitTask|> Async.StartImmediate
74+
|> Async.AwaitTask|> Async.Start
5075

51-
member__.Register(defaultSettings:'options)=
52-
defaultSettings|> updateFromStore|> write
76+
// This is the point we retrieve the initial value and subscribe to watch for changes
77+
member__.Register(defaultSettings:'options)=
78+
defaultSettings|> updateFromStore|> keepInCache
5379
letsubset= defaultSettings.GetType()|> storageKey|> settingsManager.GetSubset
54-
80+
// this event is also raised when a setting change occurs in another VS instance, so we can keep everything in sync
5581
PropertyChangedAsyncEventHandler(fun _ _->
56-
(read():'options)|> updateFromStore|>write
82+
(getCached():'options)|> updateFromStore|>keepInCache
5783
System.Threading.Tasks.Task.CompletedTask)
5884
|> subset.add_SettingChangedAsync
85+
5986

‎vsintegration/src/FSharp.Editor/Options/UIHelpers.fs‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@ module internal OptionsUIHelpers =
1313
typeAbstractOptionPage<'options>()as this=
1414
inherit UIElementDialogPage()
1515

16+
// this replicates the logic used by Roslyn option pages. Some following comments have been copied from
17+
// https://github.com/dotnet/roslyn/blob/5b125935f891b3c20405459f8f7e1cdfdc2cfa3d/src/VisualStudio/Core/Impl/Options/AbstractOptionPage.cs
18+
let mutableneedsLoadOnNextActivate=true
19+
1620
letview=lazy this.CreateView()
17-
21+
1822
letoptionService=
1923
// lazy, so GetService is called from UI thread
2024
lazy
@@ -25,11 +29,33 @@ module internal OptionsUIHelpers =
2529

2630
overridethis.Child=upcast view.Value
2731

32+
overridethis.OnActivate _=
33+
if needsLoadOnNextActivatethen
34+
// It looks like the bindings do not always pick up new source, unless we cycle the DataContext like this
35+
view.Value.DataContext<- DependencyProperty.UnsetValue
36+
view.Value.DataContext<- optionService.Value.LoadSettings<'options>()
37+
needsLoadOnNextActivate<-false
38+
2839
overridethis.SaveSettingsToStorage()=
29-
downcast view.Value.DataContext|> optionService.Value.Write<'options>
40+
downcast view.Value.DataContext|> optionService.Value.SaveSettings<'options>
41+
// Make sure we load the next time the page is activated, in case if options changed
42+
// programmatically between now and the next time the page is activated
43+
needsLoadOnNextActivate<-true
3044

31-
overridethis.LoadSettingsFromStorage()=
32-
view.Value.DataContext<- optionService.Value.Read<'options>()
45+
overridethis.LoadSettingsFromStorage()=
46+
// This gets called in two situations:
47+
//
48+
// 1) during the initial page load when you first activate the page, before OnActivate
49+
// is called.
50+
// 2) during the closing of the dialog via Cancel/close when options don't need to be
51+
// saved. The intent here is the settings get reloaded so the next time you open the
52+
// page they are properly populated.
53+
//
54+
// This second one is tricky, because we don't actually want to update our controls
55+
// right then, because they'd be wrong the next time the page opens -- it's possible
56+
// they may have been changed programmatically. Therefore, we'll set a flag so we load
57+
// next time
58+
needsLoadOnNextActivate<-true
3359

3460
//data binding helpers
3561
letradioButtonCoverter=

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp