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

Settings sync for settings GUI #84431

Closed
roblourens opened this issue Nov 10, 2019 · 9 comments · Fixed by #90190
Closed

Settings sync for settings GUI #84431

roblourens opened this issue Nov 10, 2019 · 9 comments · Fixed by #90190
Assignees
Labels
config VS Code configuration, set up issues feature-request Request for new features or functionality on-testplan settings-editor VS Code settings editor issues settings-sync
Milestone

Comments

@roblourens
Copy link
Member

We need to provide some UI for settings sync, especially for resolving simple conflicts without dropping the user into a JSON diff.

#78966

@roblourens roblourens added feature-request Request for new features or functionality config VS Code configuration, set up issues settings-editor VS Code settings editor issues labels Nov 10, 2019
@roblourens roblourens added this to the November 2019 milestone Nov 10, 2019
@roblourens roblourens self-assigned this Nov 10, 2019
@miguelsolorio
Copy link
Contributor

Here's the conceptual mockups for resolving conflicts. When we detect that there are conflicts, we will prompt the user either via a notification:

Or via the settings gear:

This will lead them to the Settings GUI (if workbench.settings.editor is set to ui) and we'll enter the @conflicts query into the search bar which will bring up the conflicts UI:

Ressolve Conflict - Large

  • If there is more than 1 conflict, we will add a "Sync All..." action at the top
  • We'll show two primary options for each setting, similar to our merge conflicts UI
  • Additionally we'll provide an option to "ignore" the conflict, which adds the setting to the "do not sync" list
  • After the user selects an option, this should remove it from the list
  • When all conflicts are resolved, we should automatically update the server

When we get to in narrow views, we'll adjust the layout and stack the conflicts:

Ressolve Conflict - Small

If strings are long and require it to wrap, we'll grow the conflict container (making sure the "Ignore Setting" action is adjusted):

image

@sanket-bhalerao
Copy link

  1. Is vscode insiders build and vscode stable going to be treated differently?
    As we can install/enable experimental extension/setting in insiders build.
  2. is there going to be separate installed/enabled extension sync for insiders and stable vscode installations?

@miguelsolorio
Copy link
Contributor

@sanket-bhalerao please see #2743 (comment) for all things related to Settings Sync, this issue is specifically for the UI in the Settings GUI.

@thernstig
Copy link
Contributor

The terms Sync Local and Sync Remote does not make it immediately obvious what they mean. Does Sync Local mean I will upload my local settings to the remote (i.e. overwrite the remote settings), or does it mean that I will synchronize my local settings with the remote (i.e. overwrite the local settings)?

I feel the terms will be interepreted differently by users.

Would e.g. writing Select or Pick or Choose this or Use this or something else instead on both sides make more sense? Because I assume that selecting either one will still sync the selected setting to the remote?

@miguelsolorio
Copy link
Contributor

@thernstig I think this is something we'll iterate on to find the right verbiage to describe this behavior. This is also why we decided to add icons to help illustrate it better. IMO, select/pick/choose doesn't bring any more clarity.

@sandy081
Copy link
Member

sandy081 commented Jan 8, 2020

@roblourens

Implemented ISettingsSyncService that can provide the state of settings sync and also can access conflicts and resolve them

export interface ISettingsSyncService extends ISynchroniser {
_serviceBrand: any;
getConflicts(): Promise<IConflictSetting[]>;
resolveConflicts(resolvedConflicts: { key: string, value: any | undefined }[]): Promise<void>;
}

@roblourens
Copy link
Member Author

Notes on the API

  • I need an event for when conflicts change, not just when we change from not having conflicts to having conflicts
  • I'm not sure about getConflicts returning a Promise. Ideally I would just listen for the event that says there are conflicts, and at that moment, there would be an array of conflicts. Since this is used in the process of rendering the settings UI, I don't want to block that on file accesses or IPC. If it's not easy for the SettingsSyncService to make that change, it can be worked around on my end. But if the SettingsSyncService can work that way, it would be slightly more convenient.

@sandy081
Copy link
Member

sandy081 commented Jan 9, 2020

@roblourens Changed it as requested

export interface ISettingsSyncService extends ISynchroniser {
_serviceBrand: any;
readonly onDidChangeConflicts: Event<IConflictSetting[]>;
readonly conflicts: IConflictSetting[];
resolveConflicts(resolvedConflicts: { key: string, value: any | undefined }[]): Promise<void>;
}

@roblourens
Copy link
Member Author

Current status, we were discussing whether or not we really need this full sophisticated GUI for resolving conflicts setting by setting. Figuring out how to communicate different situations in the settings UI gets complicated and it's feeling like not quite the right solution.

Seems that conflicts will not usually occur that often, and when they do, users will probably typically just want to accept all the local or remote conflicted settings together, or in an advanced use case, the JSON editor is probably fine for those users.

So one idea we had is that when you click "Resolve Conflicts", you would just get a list of settings that are in conflict, with buttons to "accept all local", "accept all remote", or "see details", where the first two buttons would resolve the conflicts with the local or remote versions, and the third would open the JSON editor for advanced users who want to make individual changes.

Also, I think that we still don't have a good concept of how merging keybindings conflicts will work, and maybe we shouldn't go too far down the road of designing a complicated resolution UI for settings without a better understanding of that and how the two go together.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
config VS Code configuration, set up issues feature-request Request for new features or functionality on-testplan settings-editor VS Code settings editor issues settings-sync
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants