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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Marco Bonardo [:mak]]

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.

Comment 1

[Marco Bonardo [:mak]]

Attachment: Bug 1341585 - Ignore Visual Studio Code workspace settings and add a recommended extensions file.

Review commit: https://reviewboard.mozilla.org/r/114478/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/114478/

Comment 2

[Marco Bonardo [:mak]]

Ted, do you know what are our plans here? (looks like :gps is away)

Comment 3

[Marco Bonardo [:mak]]

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/#review115904

::: .vscode/settings.json:1
(Diff revision 1)
> +// Inserire le impostazioni in questo file per sovrascrivere quelle predefinite

I should remove this.

Comment 4

[Jean-Yves Avenard [:jya]]

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.

Comment 5

[Marco Bonardo [:mak]]

(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.

Comment 6

[(not currently active) Ted Mielczarek]

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.

Comment 7

[Marco Bonardo [:mak]]

(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?

Comment 8

[(not currently active) Ted Mielczarek]

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.

Comment 9

[Marco Bonardo [:mak]]

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.

Comment 10

[Marco Bonardo [:mak]]

Comment on attachment 8839870 [details]
Bug 1341585 - Ignore Visual Studio Code workspace settings and add a recommended extensions file.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114478/diff/1-2/

Comment 11

[Marco Bonardo [:mak]]

Comment on attachment 8839870 [details]
Bug 1341585 - Ignore Visual Studio Code workspace settings and add a recommended extensions file.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114478/diff/2-3/

Comment 12

[Marco Bonardo [:mak]]

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.

Comment 13

[Marco Bonardo [:mak]]

Thread created in dev-platform and firefox-dev.

Comment 14

[Jean-Yves Avenard [:jya]]

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.

Comment 15

[Marco Bonardo [:mak]]

(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.

Comment 16

[Marco Bonardo [:mak]]

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.

Comment 17

[Jean-Yves Avenard [:jya]]

(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 18

[Marco Bonardo [:mak]]

Comment on attachment 8839870 [details]
Bug 1341585 - Ignore Visual Studio Code workspace settings and add a recommended extensions file.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114478/diff/3-4/

Comment 19

[Jean-Yves Avenard [:jya]]

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

Comment 20

[Marco Bonardo [:mak]]

ted, any news on this review? Is there anything blocking us?

Comment 21

[Gregory Szorc [:gps]]

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.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eaba1e1ad2ab
Ignore Visual Studio Code workspace settings and add a recommended extensions file. r=gps,jya

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/eaba1e1ad2ab