Skip to content

Conversation

@iamakulov
Copy link
Contributor

Summary of the Pull Request

In #3575, Terminal added support for the rowsToScroll field – which accepts integers or "system". However, the settings schema only allows integers:

"rowsToScroll": {
"default": "system",
"description": "The number of rows to scroll at a time with the mouse wheel. This will override the system setting if the value is not zero or 'system'.",
"maximum": 999,
"minimum": 0,
"type": "integer"
},

which means writing rowsToScroll: "system" produces the following warning:

image

This PR fixes the issue.

PR Checklist

  • Closes #xxx
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Validation Steps Performed

  1. Opened settings and replaced the $schema field with the updated schema from my fork (https://raw.githubusercontent.com/iamakulov/terminal/b5d4c9337d3f323613dd5209bb68decb86cce667/doc/cascadia/profiles.schema.json).
  2. Validated that the "system" value doesn’t produce the warning anymore
  3. Validated that the value of false (i.e., a boolean – a type that’s not accepted) still produces a warning

@zadjii-msft zadjii-msft added Area-Schema Things that have to do with the json schema. Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs labels Jan 24, 2020
@cinnamon-msft cinnamon-msft added the Needs-Second It's a PR that needs another sign-off label Jan 24, 2020
@DHowett-MSFT
Copy link
Contributor

thanks!

@DHowett-MSFT DHowett-MSFT merged commit f0e6037 into microsoft:master Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Schema Things that have to do with the json schema. Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Needs-Second It's a PR that needs another sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants