Closed Bug 1341585 Opened 3 years ago Closed 3 years ago

Add .vscode/settings.json to ignore files and provide a list of recommended extensions for VS Code

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mak, Assigned: mak)

References

Details

Attachments

(1 file)

I'm starting investigating the usage of VS Code for my coding, apart from a few issues (Quick Open is horribly slow), it seems to be a very good replacement for Sublime Text, and it's much faster than Atom.

I have found a lot of options are common for most of our codebase, so I'm wondering if we could support vscode workspace settings. It may help newcomers to have a properly setup editor.

Otherwise, I'd like us to ignore .vscode so I can have my own workspace definitions there (But considering bug 1323308, that sounds unlikely)

I'll attach a patch with my current workspace, for evaluation.
Ted, do you know what are our plans here? (looks like :gps is away)
Flags: needinfo?(ted)
I don't think this should go as-is.

People will tweak VS Code to their own favourite options.
Those "defaults" certainly aren't what I'm using.

So it will always shows up as having a modified .vscode content which is bad.

It should simply be added to .gitignore and not provide a default. Or at least make a default that makes sense for everyone.
(In reply to Jean-Yves Avenard [:jya] from comment #4)
> I don't think this should go as-is.

I totally agree! But it's better starting a discussion from somewhere, than not starting it at all.
And if we agree on an approach, then it's worth bringing to a broader audience (dev-platform)

> So it will always shows up as having a modified .vscode content which is bad.

Right, it would be simpler if user settings would override workspace settings, unfortunately it's the opposite. That's why we need a very short list that we could agree upon, if we take one.

> It should simply be added to .gitignore and not provide a default. Or at
> least make a default that makes sense for everyone.

Absolutely, this bug is about doing one of those 2. The blocked bug though makes so that we can't just ignore all .vscode, so we may have to ignore only certain files.

Btw, some thoughts about what I suggested, for discussion:

  "editor.tabSize": 2,
  "editor.rulers": [80],
  "files.eol": "\n",
  "files.insertFinalNewline": true,

These are mandate by the Mozilla coding style, afaict.

  "files.associations": {
    "*.jsm": "javascript"
  },

Again, I don't think anyone would disagree.

  "eslint.nodePath": "tools/lint/eslint/node_modules/.bin",

This makes us use our own eslint, that has mozilla modules. It would simplify a lot setting up eslint for newcomers.

  "workbench.activityBar.visible": true,

this was unintended... ignore it.

  "files.exclude": {
    "**/.*": true,
    "**/obj*": true
  },

Avoid searching and indexing these. Debatable.

"window.title": "${dirty}${activeFilePath}${separator}${rootName}${separator}${appName}",

Again, debatable. The only difference from the default here is that it shows the complete file path, rather then just the filename.

The extensions are only suggested, not installed by default. So there isn't big risk into providing a list of suggested extensions. We could add/remove based on how things change in code:

"dzannotti.vscode-babel-coloring",
"dbaeumer.vscode-eslint",
"dzannotti.theme-spacemacs"

These are must-have for eslint and ES6-7

"NathanRidley.autotrim",
"ybaumes.highlight-trailing-white-spaces",

mostly for whitespace sanity, one only trims lines you touch, the other one hilights trailing spaces.

"ms-vscode.cpptools",

didn't use these yet, but they look quite handy.
Summary: Either add a default .vscode, or add it to .hgignore → Either add a default .vscode/settings.json, or add it to .hgignore
I don't think we have particularly strong guidelines on editor settings in-tree. We've historically had emacs+vi modelines in many source files, and we do have a configuration file for YouCompleteMe for vim in the repository.

Is it possible to provide a set of defaults for VS Code and then allow people to override them without them modifying the checked-in file? If so, I would support that. If not, it seems like having a checked-in config file would result in more hassle for people.
Flags: needinfo?(ted)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> Is it possible to provide a set of defaults for VS Code and then allow
> people to override them without them modifying the checked-in file? If so, I
> would support that. If not, it seems like having a checked-in config file
> would result in more hassle for people.

not right now. as I said workspace settings unfortunately override user settings.

That said, even if we'd ignore settings.json (so everyone can just put his) and maybe propose some settings on mdn, di we want to consider a recommended extensions list?
If we can provide some helpful configuration for VS Code users in any form I think that's useful. It looks like rillian started some work in that direction in bug 1323308.
Ok, I think for now we could ignore settings.json (in .gitignore and .hgignore) and maybe add a list of suggested extensions. To build that list we could send a message to dev-platform and collect extensions people found useful.

From what I can tell, the list of settings that "everyone" would agree upon would be so short, that in practice it would just disallow people from putting their own settings.json in their tree .vscode/. That's a no-go.
I have updated the patch to ignore settings.json and added comments to the recommended extensions list.
I'll start a thread in dev-platform to collect ideas about extensions we are using and feedback about the current list.
Assignee: nobody → mak77
Summary: Either add a default .vscode/settings.json, or add it to .hgignore → Add .vscode/settings.json to ignore files and provide a list of recommended extensions for VS Code
Thread created in dev-platform and firefox-dev.
Comment on attachment 8839870 [details]
Bug 1341585 - Ignore Visual Studio Code workspace settings and add a recommended extensions file.

https://reviewboard.mozilla.org/r/114478/#review116454

::: .gitignore:127
(Diff revision 3)
>  
>  # tup database
>  /.tup
> +
> +# Ignore Visual Studio Code workspace settings file.
> +.vscode/settings.json

there are plenty of files in .vscode that needs ignoring, not just the settings.

This includes:
Launch configuration
C++ configuration
Tasks configuration.

Additionally, that's where the index files are residing.

Really, you want to ignore all of them because they will be modified by the user.
(In reply to Jean-Yves Avenard [:jya] from comment #14)
> Really, you want to ignore all of them because they will be modified by the
> user.

See bug 1323308, that is already planning to add a default tasks.json and c_cpp_properties.json
I can see users may want to add further stuff, but then it can be added when it's the time.
The only way to ignore the whole .vscode would be for us to completely give up on adding any default setting, and that's not something I want to decide.

Also, we can always do that in the future, I'm not disallowing anything with my change.
Ehr, are the index files automatically created in the workspace .vscode by the editor? Since then we really need a way to ignore everything BUT the files bug 1323308 will add.
(In reply to Marco Bonardo [::mak] from comment #16)
> Ehr, are the index files automatically created in the workspace .vscode by
> the editor? Since then we really need a way to ignore everything BUT the
> files bug 1323308 will add.

indeed:
jyapro:.vscode jyavenard$ ls -la
total 1023408
drwxr-xr-x@  2 jyavenard  staff        306 16 Feb 22:03 .
drwxr-xr-x@ 58 jyavenard  staff       3706 16 Feb 22:48 ..
-rw-r--r--@  2 jyavenard  staff  518852608 16 Feb 22:03 .browse.VC.db
-rw-r--r--@  2 jyavenard  staff      32768 16 Feb 22:03 .browse.VC.db-shm
-rw-r--r--@  2 jyavenard  staff    5079992 16 Feb 22:03 .browse.VC.db-wal
-rw-r--r--@  2 jyavenard  staff       1258 16 Feb 22:03 c_cpp_properties.json
-rw-r--r--@  3 jyavenard  staff       2253 16 Feb 22:03 launch.json
-rw-r--r--@  4 jyavenard  staff        528 16 Feb 22:03 settings.json
-rw-r--r--@  5 jyavenard  staff       1022 16 Feb 22:03 tasks.json
jyapro:.vscode jyavenard$
Comment on attachment 8839870 [details]
Bug 1341585 - Ignore Visual Studio Code workspace settings and add a recommended extensions file.

https://reviewboard.mozilla.org/r/114478/#review118246

LGTM
Attachment #8839870 - Flags: review?(jyavenard) → review+
ted, any news on this review? Is there anything blocking us?
Comment on attachment 8839870 [details]
Bug 1341585 - Ignore Visual Studio Code workspace settings and add a recommended extensions file.

https://reviewboard.mozilla.org/r/114478/#review128154

This seems like a good first implementation. It can always be itereated on.
Attachment #8839870 - Flags: review+
Attachment #8839870 - Flags: review?(ted)
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/eaba1e1ad2ab
Ignore Visual Studio Code workspace settings and add a recommended extensions file. r=gps,jya
https://hg.mozilla.org/mozilla-central/rev/eaba1e1ad2ab
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
See Also: → 1482693
You need to log in before you can comment on or make changes to this bug.