Conversation
Signed-off-by: Yaohai Zheng <yaozheng@microsoft.com>
fbricon
left a comment
fbricon
left a comment
There was a problem hiding this comment.
- maybe we should display a warning on the 1st startup of the extension, asking the user to enable that feature
- if the feature is disabled globally, then you manually enable it, the files are not excluded until the next vscode restart.
package.json
Outdated
| "java.configuration.excludeProjectSettingFiles": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Exclude the extension generated project setting files from file explorer.", |
There was a problem hiding this comment.
settings from the file explorer
There was a problem hiding this comment.
need to explain setting it to false won't remove the exclusions
src/setting.ts
Outdated
| import { getJavaConfiguration } from './utils'; | ||
|
|
||
|
|
||
| const DEFAULT_HIIDEN_FILES: string[] = ['**/.classpath', '**/.project', '**/.settings']; |
package.json
Outdated
| "description": "Specifies the severity of the message when the classpath is incomplete for a Java file", | ||
| "scope": "window" | ||
| }, | ||
| "java.configuration.excludeProjectSettingFiles": { |
src/setting.ts
Outdated
| }); | ||
| } | ||
|
|
||
| export function excludeProjectSettingFiles(workspaceUri: Uri) { |
There was a problem hiding this comment.
excludeProjectSettingsFiles
"files.exclude": {
"**/.git": true,
"**/.svn": true,
"**/.hg": true,
"**/CVS": true,
"**/.DS_Store": true,
"**/.classpath": true,
"**/.project": true,
"**/.settings": true
}is added to the local settings.json, even if the java exclusions are already defined at the user settings level. That's a big no no |
|
@fbricon Addressed the review feedback. |
Signed-off-by: Yaohai Zheng <yaozheng@microsoft.com>
src/setting.ts
Outdated
| @@ -0,0 +1,69 @@ | |||
| 'use strict'; | |||
|
you also need to hide |
|
if your settings contains agreeing to hide the project settings will change the settings to |
|
For setting generated, if user choose the workspace or global, the settings is split. Should we always keep them the same place? I would let the user to make the decision here. |
Signed-off-by: vscjava <vscjava@vscjavas-MacBook-Pro.local>
|
@fbricon Updated |
|
How about we change the choices to
If files exclusions exists in the workspace preferences, then we don't propose to "Exclude globally" |
src/settings.ts
Outdated
| let needUpdate = false; | ||
| for (const hiddenFiles of DEFAULT_HIDDEN_FILES) { | ||
| if (!excludedValue.hasOwnProperty(hiddenFiles)) { | ||
| needExcludeFiles[hiddenFiles] = true; |
There was a problem hiding this comment.
no, this will always store a subset of the files, overriding existing settings.
We do want to merge excludedValue + DEFAULT_HIDDEN_FILES in config.get('exclude')
| if (!params.affectsConfiguration('java')) { | ||
| return; | ||
| } | ||
| let newConfig = getJavaConfiguration(); |
There was a problem hiding this comment.
we want to check that if configuration.excludeProjectSettingsFiles is true, but it was false before, then excludeProjectSettingsFiles should be called.
Be aware that oldConfig is only changed if the "java configuration" changed, so it makes things a bit trickier.
Would keep the term simple and consistent with other messages in the similar situation. |
|
I'm fine with having better labels, but they need to be explicit |
|
@fbricon Updated PR. |
|
@yaohaizh I've renamed the new preference to better reflect its role: |
Signed-off-by: Yaohai Zheng <yaozheng@microsoft.com>
2 participants
@fbricon
Fix issue #618 which gives us lots of negative feedback and requested by the users.
Since this PR microsoft/vscode#66451 is closed by VSCode side, provide the vscode java setting to mitigate this issue.