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

Add extension setting field for indicating it may be influenced by a deprecated/renamed setting #91618

Closed
Colengms opened this issue Feb 26, 2020 · 11 comments
Assignees
Labels
feature-request Request for new features or functionality settings-editor VS Code settings editor issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@Colengms
Copy link
Contributor

We'd like to rename some settings in our extension, without having to rewrite users settings files. We'd like to use deprecationMessage, and would like to fall back to the old setting if the new setting is not set (by using WorkspaceConfiguration.inspect()). I believe we would need a minor addition in order to make the settings UI more user-friendly when renaming settings and falling back to old settings when not present.

Could we get an additional field in package.json for settings (configuration.properties), with which we can specify a deprecated setting that may influence the (new) setting if unset/default? In settings UI, display an additional message for the new setting indicating its effective value may be influenced by a deprecated setting (and perhaps a link to the deprecated setting, and a button to remove the deprecated setting value). For example, in the case below the effective value of the setting will be "Enabled" because the old setting is set to "Enabled", and the new setting is unset (It's default is "EnabledIfIncludesResolve", which is displayed also when the setting is unset, but is not the setting we would use).

image

image

Showing this message is a simple solution. A more complicated solution might be to somehow reflect in the new setting what the effective value is when unset. But, it's conceivable that a newer setting does not map directly to a deprecated setting, or perhaps multiple deprecated settings may influence the behavior of a new setting, or there is otherwise not a clear relationship between old and new settings.

You might also consider not showing a deprecated setting in settings UI when there is an associated newer setting (and it is set to something). And, perhaps store the default value (instead of clearing out the value, which is the current behavior) if a newer setting (with associated deprecated setting(s)) becomes set to its default value via the UI.

I might be over thinking this, but I've tried a number of approaches. If there is a proper way to deprecate settings and fall back to them when new settings are not present, that does not break the setting UI experience, please let me know.

@vscodebot
Copy link

vscodebot bot commented Feb 26, 2020

@roblourens
Copy link
Member

You could add that message to the normal setting description, right? Or do you want it to only show up when the deprecated setting is set?

What I would do here is set the new setting to have the value that does whatever the deprecated setting does, then remove the deprecated setting. This would let you make a clean break and get the user onto the right setting. But I think we have used a variety of strategies in vscode, including getting rid of the deprecated setting with no automatic migration to a new setting (especially when behavior has changed and it's hard to know what the user will now want) and doing the same as what you want to do except with no message pointing from the new setting to the old setting.

@roblourens roblourens added the info-needed Issue requires more information from poster label Feb 27, 2020
@roblourens roblourens self-assigned this Feb 27, 2020
@Colengms
Copy link
Contributor Author

You could add that message to the normal setting description, right? Or do you want it to only show up when the deprecated setting is set?

Yes to both. We could explain in the description of the new setting that a deprecated setting may actually be used despite the new setting being displayed as set to default value in settings UI. But, it would distract from the setting description, and it would seem better if a message to that effect were only displayed when it's an issue that should be addressed, and highlighted as such. (Difficulty with configuration is our top user-reported issue).

What I would do here is set the new setting to have the value that does whatever the deprecated setting does, then remove the deprecated setting.

We were trying to go the deprecationMessage route, but may need to go the rewrite-settings route to avoid confusion in settings UI. There were also concerns about what happens if reverting to a prior version of our extension, or users sharing or checking in settings files and not all users using the same version. If something like this suggestion were available, we'd prefer to allow the user to own modification of their own settings.

@roblourens roblourens added feature-request Request for new features or functionality settings-editor VS Code settings editor issues and removed info-needed Issue requires more information from poster labels Feb 27, 2020
@roblourens roblourens added this to the Backlog Candidates milestone Feb 27, 2020
@vscodebot
Copy link

vscodebot bot commented Feb 27, 2020

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@roblourens
Copy link
Member

a deprecated setting may actually be used despite the new setting being displayed as set to default value in settings UI

Keep in mind, there are other cases where the value displayed may not be the one that is used right now, it can be overridden by workspace or folder settings, or language-specific setting overrides. So in theory, users should be used to thinking of the settings UI that way, but the info we want to convey is complicated.

@roblourens
Copy link
Member

roblourens commented Apr 18, 2020

Planning to do for this month:

  • Adding a link to another setting in the deprecation message
  • Fix some minor issues around clicking those setting links

leilapearson pushed a commit to relmify/vscode that referenced this issue Apr 27, 2020
Fix bug with search + clicking setting name links
Fix microsoft#91618
@roblourens roblourens added the verification-needed Verification of issue is requested label Apr 27, 2020
@roblourens
Copy link
Member

Verify

  • markdownDeprecationMessage exists, and when set, will appear rendered as markdown in the settings UI but as plaintext elsewhere
  • deprecationMessage can also be set at the same time, and will be used in hover/problems view while the markdown version is used in the settings UI
  • clicking setting links in deprecationMessages works

@Colengms
Copy link
Contributor Author

@roblourens The recent changes do not address the issue this was opened to track. Could you please re-open?

We'd like to be able to rename some settings and provide backwards compatibility for the old settings names for some time. Doing this would seem to involve using the old setting if explicitly set when the new setting is not explicitly set (is undefined).

In a nutshell, the issue is apparent in the following scenario:

        "test-old": {
          "type": "boolean",
          "default": false,
          "description": "The old setting",
          "deprecationMessage": "This setting is deprecated",
          "scope": "resource"
        },
        "test-new": {
          "type": "boolean",
          "default": false,
          "description": "The new setting",
          "scope": "resource"
        }

When settings are set like so:

    "test-old": true

The effective setting in the above case is: true

However, in settings UI, the following is displayed to the user: (apparent false)

image

The issue is that the new setting appears to be disabled. The old setting (in this case the equivalent setting, just renamed) is still present, so takes precedence over the non-existing new setting. UI for the old setting may not be displayed anywhere near the new setting in the UI. This is further complicated by the fact that the settings UI will undefine a setting when the user sets it to its default value.

The ask here is for a message on the new setting indicating that an old setting may actually be taking precedence, and link to that old setting. It wouldn't be necessary to display that message all the time. Rather, it could be only displayed when there is a value for the old setting, and the new setting is not defined.

i.e.:

        "test-old": {
          "type": "boolean",
          "default": false,
          "description": "The old setting",
          "deprecationMessage": "This setting is deprecated",
          "scope": "resource"
        },
        "test-new": {
          "type": "boolean",
          "default": false,
          "description": "The new setting",
          "deprecates": [ "test-old" ],
          "scope": "resource"
        }

@Colengms
Copy link
Contributor Author

Note: The reason it's not ideal to put a "markdownDeprecationMessage" on the new setting is that it marks the new setting as deprecated as well. So, the settings UI does not display an entry for it if not already explicitly set.

@roblourens
Copy link
Member

I understand what you were asking for, but I don't think it makes sense for us to support that use case. I'm hoping that #91618 (comment) will help a bit, sorry that I can't do more.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality settings-editor VS Code settings editor issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants
@roblourens @jrieken @Colengms and others