Add user settings in issue template by paulacamargo25 · Pull Request #16492 · microsoft/vscode-python

paulacamargo25 · 2021-06-16T20:59:20Z

Add user setting in issue template.

Closes #231.

karthiknadig · 2021-06-21T19:50:51Z

@paulacamargo25 Are you working on this? I assume you are handing the string setting cases?

paulacamargo25 · 2021-06-22T18:08:18Z

@paulacamargo25 Are you working on this? I assume you are handing the string setting cases?

Yes, now its done :D

karrtikr

I am unclear on what false and "placeholder" means in the JSON file, seems like we're handling that in the same way.

cc @karthiknadig

karrtikr · 2021-06-22T18:20:05Z

+        "Flake8Args": true,
+        "flake8CategorySeverity": false,
+        "flake8Enabled": true,
+        "flake8Path": false,


Not clear on why we have marked this as false, but marked banditPath or like as "placeholder". They both seem similar to me.

karrtikr · 2021-06-22T18:20:39Z

+    "linting": {
+        "enabled": true,
+        "cwd": true,
+        "Flake8Args": true,


Suggested change

"Flake8Args": true,

"flake8Args": "placeholder",

Should be lowercased.

brettcannon

I realized we can't take any arguments to tools directly as they may contain file paths.

brettcannon · 2021-06-22T20:31:05Z

+    "initialize": false,
+    "pythonPath": false,
+    "onDidChange": false,
+    "defaultInterpreterPath": false,


Suggested change

"defaultInterpreterPath": false,

"defaultInterpreterPath": "placeholder",

brettcannon · 2021-06-22T20:31:31Z

+    "downloadLanguageServer": true,
+    "jediPath": false,
+    "jediMemoryLimit": false,
+    "envFile": false,


Suggested change

"envFile": false,

"envFile": "placeholder",

brettcannon · 2021-06-22T20:31:49Z

+    "envFile": false,
+    "venvPath": "placeholder",
+    "venvFolders": false,
+    "condaPath": false,


Suggested change

"condaPath": false,

"condaPath": "placeholder",

brettcannon · 2021-06-22T20:32:17Z

+    "venvPath": "placeholder",
+    "venvFolders": false,
+    "condaPath": false,
+    "pipenvPath": false,


Suggested change

"pipenvPath": false,

"pipenvPath": "placeholder",

brettcannon · 2021-06-22T20:32:26Z

+    "venvFolders": false,
+    "condaPath": false,
+    "pipenvPath": false,
+    "poetryPath": false,


Suggested change

"poetryPath": false,

"poetryPath": "placeholder",

brettcannon · 2021-06-22T20:36:27Z

+    "testing": {
+        "cwd": true,
+        "debugPort": true,
+        "nosetestArgs": true,


Suggested change

"nosetestArgs": true,

"nosetestArgs": "placeholder",

brettcannon · 2021-06-22T20:36:34Z

+        "nosetestsEnabled": true,
+        "nosetestPath": "placeholder",
+        "promptToConfigure": true,
+        "pytestArgs": true,


Suggested change

"pytestArgs": true,

"pytestArgs": "placeholder",

brettcannon · 2021-06-22T20:36:43Z

+        "pytestArgs": true,
+        "pytestEnabled": true,
+        "pytestPath": "placeholder",
+        "unittestArgs": true,


Suggested change

"unittestArgs": true,

"unittestArgs": "placeholder",

brettcannon · 2021-06-22T20:38:44Z

+        "autoTestDiscoverOnSaveEnabled": true
+    },
+    "terminal": true,
+    "tensorBoard": true,


The only setting related to this that I could find is "python.tensorBoard.logDirectory", which should be a placeholder.

brettcannon · 2021-06-22T20:39:31Z

+        "unittestEnabled": true,
+        "autoTestDiscoverOnSaveEnabled": true
+    },
+    "terminal": true,


There's a bunch of subcommands.

vscode-python/package.json

Lines 1746 to 1769 in 67590fb

"python.terminal.activateEnvironment": {

"type": "boolean",

"default": true,

"description": "Activate Python Environment in Terminal created using the Extension.",

"scope": "resource"

},

"python.terminal.executeInFileDir": {

"type": "boolean",

"default": false,

"description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.",

"scope": "resource"

},

"python.terminal.launchArgs": {

"type": "array",

"default": [],

"description": "Python launch arguments to use when executing a file in the terminal.",

"scope": "resource"

},

"python.terminal.activateEnvInCurrentTerminal": {

"type": "boolean",

"default": false,

"description": "Activate Python Environment in the current Terminal on load of the Extension.",

"scope": "resource"

},

I guess python.terminal.launchArgs could also take file paths and should be marked as a placeholder, when the list is expanded.

brettcannon · 2021-06-22T20:45:27Z

Would it make sense to either have the allowlist only worry about non-enum strings and arrays from

vscode-python/package.json

Line 1056 in 67590fb

"properties": {

so that it's easier to keep up-to-date? Or have a Python script that reads package.json and adds any missing fields with an appropriate marker (either "placeholder" to be safe or XXX to know what changed), and remove any settings that got removed from the "python" namespace?

karrtikr · 2021-06-22T21:37:09Z

@@ -0,0 +1,126 @@
+{
+    "initialize": false,
+    "pythonPath": false,


Suggested change

"pythonPath": false,

"pythonPath": "placeholder",

karrtikr · 2021-06-22T21:37:51Z

+    "jediMemoryLimit": false,
+    "envFile": false,
+    "venvPath": "placeholder",
+    "venvFolders": false,


Suggested change

"venvFolders": false,

"venvFolders": "placeholder",

karrtikr · 2021-06-22T21:39:02Z

+    "workspaceRoot": false,
+    "linting": {
+        "enabled": true,
+        "cwd": true,


Suggested change

"cwd": true,

"cwd": "placeholder",

karrtikr · 2021-06-22T21:43:33Z

+    "defaultLS": {
+        "defaultLSType": true
+    },


Suggested change

"defaultLS": {

"defaultLSType": true

},

"defaultLS": true,

If all subsettings are marked as true, this should work as well.

karrtikr · 2021-06-22T21:44:44Z

+    "autoUpdateLanguageServer": false,
+    "languageServer": true,
+    "languageServerIsDefault": false,
+    "logging": false,


Suggested change

"logging": false,

"logging": true,

karrtikr · 2021-06-22T21:46:11Z

+        "flake8Path": false,
+        "ignorePatterns": false,
+        "lintOnSave": true,
+        "maxNumberOfProblems": false,


Suggested change

"maxNumberOfProblems": false,

"maxNumberOfProblems": true,

This has not been addressed yet.

karrtikr · 2021-06-22T21:53:10Z

Would it make sense to either have the allowlist only worry about non-enum strings and arrays from package.json so that it's easier to keep up-to-date?

I like this idea. All of the placeholder items are either non-enum strings or arrays. We could have an exception list instead, where we allow certain non-enum strings or arrays. For eg. python.experiments.optInto is an array but should not be a placeholder.

brettcannon · 2021-06-23T20:11:48Z

All of the placeholder items are either non-enum strings or arrays. We could have an exception list instead, where we allow certain non-enum strings or arrays. For eg. python.experiments.optInto is an array but should not be a placeholder.

Yep, I think we are saying the same thing with different terminology 😄: read package.json, and if it's a non-enum string or array just use "placeholder" and only swap out if some allowlist/exceptions list specifies that the setting is safe to include as-is.

brettcannon · 2021-07-13T17:00:40Z

Is this held up on a positive review from @karrtikr at this point?

karrtikr

@brettcannon I didn't get the memo I was not re-requested for review.

LGTM otherwise.

I guess we're not going with Brett's suggestion earlier #16492 (comment)?

karrtikr · 2021-07-14T20:19:48Z

+        "flake8Path": false,
+        "ignorePatterns": false,
+        "lintOnSave": true,
+        "maxNumberOfProblems": false,


This has not been addressed yet.

brettcannon · 2021-07-16T18:22:44Z

@karrtikr I don't think there's a need to automate this right now since we don't mutate our settings often enough (i.e. potentially a premature optimization).

github-actions Bot assigned paulacamargo25 Jun 16, 2021

karthiknadig self-requested a review June 16, 2021 21:01

karthiknadig force-pushed the Add-user-settings branch from 7ef6de1 to 802dadf Compare June 17, 2021 06:06

paulacamargo25 added the no-changelog No news entry required label Jun 17, 2021

paulacamargo25 marked this pull request as ready for review June 17, 2021 20:09

github-actions Bot requested a review from karrtikr June 17, 2021 20:09

paulacamargo25 and others added 3 commits June 22, 2021 13:04

Add user settings in issue template

248f8ab

Fix tests

1c52fa8

Add placeholder for path settings

b13486f

karthiknadig approved these changes Jun 22, 2021

View reviewed changes

karrtikr suggested changes Jun 22, 2021

View reviewed changes

brettcannon suggested changes Jun 22, 2021

View reviewed changes

karrtikr reviewed Jun 22, 2021

View reviewed changes

Fix user settings json values

47ca7fa

paulacamargo25 force-pushed the Add-user-settings branch from a18484e to 47ca7fa Compare June 24, 2021 20:45

brettcannon self-requested a review June 25, 2021 17:02

Fix tests

2b1496d

brettcannon requested a review from karrtikr July 13, 2021 17:00

karrtikr approved these changes Jul 14, 2021

View reviewed changes

brettcannon requested review from brettcannon and removed request for brettcannon July 16, 2021 18:23

brettcannon approved these changes Jul 16, 2021

View reviewed changes

paulacamargo25 merged commit 28be0f4 into microsoft:main Jul 19, 2021

	"defaultInterpreterPath": false,
	"defaultInterpreterPath": "placeholder",

	"python.terminal.activateEnvironment": {
	"type": "boolean",
	"default": true,
	"description": "Activate Python Environment in Terminal created using the Extension.",
	"scope": "resource"
	},
	"python.terminal.executeInFileDir": {
	"type": "boolean",
	"default": false,
	"description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.",
	"scope": "resource"
	},
	"python.terminal.launchArgs": {
	"type": "array",
	"default": [],
	"description": "Python launch arguments to use when executing a file in the terminal.",
	"scope": "resource"
	},
	"python.terminal.activateEnvInCurrentTerminal": {
	"type": "boolean",
	"default": false,
	"description": "Activate Python Environment in the current Terminal on load of the Extension.",
	"scope": "resource"
	},

Conversation

paulacamargo25 commented Jun 16, 2021 • edited by brettcannon Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

karthiknadig commented Jun 21, 2021

Uh oh!

paulacamargo25 commented Jun 22, 2021

Uh oh!

karrtikr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karrtikr Jun 22, 2021 • edited by brettcannon Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brettcannon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettcannon commented Jun 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karrtikr commented Jun 22, 2021

Uh oh!

brettcannon commented Jun 23, 2021

Uh oh!

brettcannon commented Jul 13, 2021

Uh oh!

karrtikr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettcannon commented Jul 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

paulacamargo25 commented Jun 16, 2021 •

edited by brettcannon

Loading

karrtikr Jun 22, 2021 •

edited by brettcannon

Loading