Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added project-specific Zed IDE settings #127793

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ChaiTRex
Copy link
Contributor

This repository currently has project-specific VS Code IDE settings in .vscode and compiler/rustc_codegen_cranelift/.vscode. Now there are equivalent project-specific Zed IDE settings alongside those.

This fixes rust-analyzer not being able to properly handle this project.

Note that:

  1. The contents of src/tools/rust-analyzer/.vscode could not be translated to Zed, as they aren't basic IDE settings.

  2. One of the VS Code settings in .vscode has no corresponding setting in Zed, and so this has been noted like this:

      "_settings_only_in_vs_code_not_yet_in_zed": {
        "git.detectSubmodulesLimit": 20
      },

@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2024

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 16, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2024

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@tgross35
Copy link
Contributor

I don't think this repo has a top-level .vscode, but it seems like x can generate that for you https://rustc-dev-guide.rust-lang.org/building/suggested.html#configuring-rust-analyzer-for-rustc. Maybe we could do the same here?

@rust-log-analyzer

This comment has been minimized.

@nnethercote
Copy link
Contributor

This repository currently has project-specific VS Code IDE settings in .vscode and compiler/rustc_codegen_cranelift/.vscode.

.vscode doesn't exist but compiler/rustc_codegen_cranelift/.vscode does exist. That's strange. (cc @bjorn3)

I don't know much about IDEs, but @tgross35's suggestion about generating the Zed settings seems reasonable.

@bjorn3
Copy link
Member

bjorn3 commented Jul 16, 2024

As @tgross35 said, for this repo, ./x.py has a command to generate .vscode/settings.json as well as update it when the user hasn't changed it themself (by keeping hashes of all previous versions). For Zed we should probably do something similar. As for compiler/rustc_codegen_cranelift, that is maintained in a separate repo (https://github.com/rust-lang/rustc_codegen_cranelift) which is periodically synced with this repo. It doesn't have a command for generating .vscode/settings.json. Instead I checked in my personal workspace settings, which should work fine for most people.

@ChaiTRex
Copy link
Contributor Author

OK, I'll examine x.py and rust-lang/rustc_codegen_cranelift and try to get it working through those.

@ChaiTRex
Copy link
Contributor Author

@bjorn3 said:

As for compiler/rustc_codegen_cranelift, that is maintained in a separate repo (https://github.com/rust-lang/rustc_codegen_cranelift) which is periodically synced with this repo. It doesn't have a command for generating .vscode/settings.json. Instead I checked in my personal workspace settings, which should work fine for most people.

Made rustc_codegen_cranelift pull request at rust-lang/rustc_codegen_cranelift#1517.

@ChaiTRex
Copy link
Contributor Author

@rustbot
Copy link
Collaborator

rustbot commented Jul 17, 2024

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 17, 2024
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Getting action download info
Download action repository 'msys2/setup-msys2@v2.22.0' (SHA:cc11e9188b693c2b100158c3322424c4cc1dadea)
Download action repository 'actions/checkout@v4' (SHA:692973e3d937129bcbf40652eb9f2f61becf3332)
Download action repository 'actions/upload-artifact@v4' (SHA:0b2256b8c012f0828dc542b3febcab082c67f72b)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt \
    && pip3 install virtualenv
COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---

#12 [5/8] COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
#12 DONE 0.0s

#13 [6/8] RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt     && pip3 install virtualenv
#13 0.469   Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB)
#13 0.487 Collecting boolean-py==4.0
#13 0.495   Downloading boolean.py-4.0-py3-none-any.whl (25 kB)
#13 0.513 Collecting chardet==5.1.0
---
#13 3.845 Building wheels for collected packages: reuse
#13 3.846   Building wheel for reuse (pyproject.toml): started
#13 4.201   Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 4.202   Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269
#13 4.202   Stored in directory: /tmp/pip-ephem-wheel-cache-8yd_j91e/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
#13 4.205 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
#13 4.229   Attempting uninstall: setuptools
#13 4.229     Found existing installation: setuptools 59.6.0
#13 4.231     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
---
#13 5.623   Downloading virtualenv-20.26.3-py3-none-any.whl (5.7 MB)
#13 5.850      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 5.7/5.7 MB 25.4 MB/s eta 0:00:00
#13 5.913 Collecting filelock<4,>=3.12.2
#13 5.921   Downloading filelock-3.15.4-py3-none-any.whl (16 kB)
#13 5.943 Collecting distlib<1,>=0.3.7
#13 5.951   Downloading distlib-0.3.8-py2.py3-none-any.whl (468 kB)
#13 5.963      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 468.9/468.9 KB 51.7 MB/s eta 0:00:00
#13 5.998 Collecting platformdirs<5,>=3.9.1
#13 6.006   Downloading platformdirs-4.2.2-py3-none-any.whl (18 kB)
#13 6.100 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#13 6.292 Successfully installed distlib-0.3.8 filelock-3.15.4 platformdirs-4.2.2 virtualenv-20.26.3
#13 DONE 6.4s

#14 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#14 DONE 0.0s
---
DirectMap4k:      200640 kB
DirectMap2M:     7139328 kB
DirectMap1G:    11534336 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/08cdc2fa1a9fd5ba466838f69cfaf062a3a64ad1/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/llvm-08cdc2fa1a9fd5ba466838f69cfaf062a3a64ad1-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm
---
   Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
    Finished `release` profile [optimized] target(s) in 30.58s
##[endgroup]
fmt check
##[error]Diff in /checkout/src/bootstrap/src/core/build_steps/setup.rs at line 685:
         "\nx.py can automatically install the recommended `.zed/settings.json` file for rustc development"
     match mismatched_settings {
-        Some(true) => eprintln!(
-        Some(true) => eprintln!(
-            "WARNING: existing `.zed/settings.json` is out of date, x.py will update it"
+        Some(true) => {
+        Some(true) => {
+            eprintln!("WARNING: existing `.zed/settings.json` is out of date, x.py will update it")
         Some(false) => eprintln!(
         Some(false) => eprintln!(
             "WARNING: existing `.zed/settings.json` has been modified by user, x.py will back it up and replace it"
         ),
##[error]Diff in /checkout/src/bootstrap/src/core/builder.rs at line 992:
                 run::GenerateWindowsSys,
                 run::GenerateCompletions,
             ),
-            Kind::Setup => describe!(setup::Profile, setup::Hook, setup::Link, setup::VsCode, setup::Zed),
+            Kind::Setup => {
+                describe!(setup::Profile, setup::Hook, setup::Link, setup::VsCode, setup::Zed)
+            }
             Kind::Clean => describe!(clean::CleanAll, clean::Rustc, clean::Std),
             Kind::Vendor => describe!(vendor::Vendor),
             // special-cased in Build::build()
fmt error: Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/rustfmt/bin/rustfmt" "--config-path" "/checkout" "--edition" "2021" "--unstable-features" "--skip-children" "--check" "/checkout/compiler/rustc_query_system/src/ich/impls_syntax.rs" "/checkout/compiler/rustc_query_system/src/ich/mod.rs" "/checkout/compiler/rustc_query_system/src/cache.rs" "/checkout/compiler/rustc_query_system/src/values.rs" "/checkout/compiler/rustc_query_system/src/query/caches.rs" "/checkout/compiler/rustc_query_system/src/query/config.rs" "/checkout/compiler/rustc_query_system/src/query/plumbing.rs" "/checkout/compiler/rustc_query_system/src/query/job.rs" "/checkout/compiler/rustc_query_system/src/query/mod.rs" "/checkout/compiler/rustc_query_system/src/lib.rs" "/checkout/src/bootstrap/src/core/build_steps/vendor.rs" "/checkout/src/bootstrap/src/core/build_steps/run.rs" "/checkout/src/bootstrap/src/core/build_steps/clean.rs" "/checkout/src/bootstrap/src/core/build_steps/toolstate.rs" "/checkout/src/bootstrap/src/core/build_steps/clippy.rs" "/checkout/src/bootstrap/src/core/build_steps/tool.rs" "/checkout/src/bootstrap/src/core/build_steps/compile.rs" "/checkout/src/bootstrap/src/core/build_steps/synthetic_targets.rs" "/checkout/src/bootstrap/src/core/build_steps/setup/tests.rs" "/checkout/src/bootstrap/src/core/build_steps/perf.rs" "/checkout/src/bootstrap/src/core/build_steps/install.rs" "/checkout/src/bootstrap/src/core/build_steps/test.rs" "/checkout/src/bootstrap/src/core/build_steps/doc.rs" "/checkout/src/bootstrap/src/core/build_steps/llvm.rs" "/checkout/src/bootstrap/src/core/build_steps/suggest.rs" "/checkout/src/bootstrap/src/core/build_steps/setup.rs" "/checkout/src/bootstrap/src/core/build_steps/format.rs" "/checkout/compiler/rustc_query_system/src/dep_graph/query.rs" "/checkout/compiler/rustc_query_system/src/dep_graph/serialized.rs" "/checkout/src/bootstrap/src/core/build_steps/check.rs" "/checkout/compiler/rustc_query_system/src/dep_graph/dep_node.rs" "/checkout/src/bootstrap/src/core/build_steps/dist.rs" "/checkout/compiler/rustc_query_system/src/dep_graph/edges.rs" "/checkout/src/bootstrap/src/core/build_steps/mod.rs" "/checkout/compiler/rustc_query_system/src/dep_graph/graph.rs" "/checkout/compiler/rustc_query_system/src/dep_graph/debug.rs" "/checkout/src/bootstrap/src/core/sanity.rs" "/checkout/compiler/rustc_query_system/src/dep_graph/mod.rs" "/checkout/src/bootstrap/src/core/metadata.rs" "/checkout/src/bootstrap/src/core/builder/tests.rs" "/checkout/src/bootstrap/src/core/download.rs" "/checkout/src/bootstrap/src/core/config/config.rs" "/checkout/src/bootstrap/src/core/config/flags.rs" "/checkout/src/bootstrap/src/core/config/tests.rs" "/checkout/src/bootstrap/src/core/config/mod.rs" "/checkout/src/bootstrap/src/core/builder.rs" "/checkout/src/bootstrap/src/core/mod.rs" "/checkout/src/bootstrap/build.rs" "/checkout/compiler/rustc_resolve/src/effective_visibilities.rs" "/checkout/compiler/rustc_resolve/src/rustdoc.rs" "/checkout/compiler/rustc_resolve/src/macros.rs" "/checkout/compiler/rustc_resolve/src/check_unused.rs" "/checkout/compiler/rustc_resolve/src/late/diagnostics.rs" "/checkout/compiler/rustc_resolve/src/ident.rs" "/checkout/compiler/rustc_resolve/src/imports.rs" "/checkout/compiler/rustc_resolve/src/lib.rs" "/checkout/compiler/rustc_resolve/src/late.rs" "/checkout/compiler/rustc_resolve/src/build_reduced_graph.rs" "/checkout/compiler/rustc_resolve/src/errors.rs" "/checkout/compiler/rustc_resolve/src/def_collector.rs" "/checkout/compiler/rustc_resolve/src/diagnostics.rs" "/checkout/compiler/rustc_ast_passes/src/ast_validation.rs" "/checkout/compiler/rustc_ast_passes/src/lib.rs" "/checkout/compiler/rustc_query_system/src/ich/hcx.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.
  local time: Wed Jul 17 16:36:09 UTC 2024
  network time: Wed, 17 Jul 2024 16:36:09 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@@ -0,0 +1,48 @@
{
// The following commented-out VS Code settings are not supported by Zed yet.
// "git.detectSubmodulesLimit": 20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option was introduced for vscode in #120138. There is a fair chance that zed won't suffer from the same issue once it has a git ui anyway (they may not even add a hard limit on the amount of auto detected git submodules at all). I don't think there is any value in keeping this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be nicer to still keep one file for RA settings, then just delete this key via bootstrap (serde_json is already a dependency). So if there are future updates that work for both, they only need to happen in one file.

Copy link
Contributor Author

@ChaiTRex ChaiTRex Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to do this properly, I think we'll need three files: one shared file for rust-analyzer settings and one file each for the non-rust-analyzer IDE-specific settings.

Even IDE settings that do the same thing are different between the two IDEs. For example, to turn on format-on-save, VS Code wants "editor.formatOnSave": true, while Zed wants "format_on_save": "on",.

There are also JSON structure requirements that differ between the IDEs. For example, VS Code wants the rust-analyzer settings to use the standard dotted notation at the root of the JSON tree, like:

{
    "rust-analyzer.rustc.source": "discover"
}

Zed wants the rust-analyzer settings to live under a path through the JSON tree of "lsp", then "rust-analyzer", then "initialization_options", then the setting with the original dots signifying an increase in tree depth:

{
  "lsp": {
    "rust-analyzer": {
      "initialization_options": {
        "rustc": {
          "source": "discover"
        }
      }
    }
  }
}

So this means that we need to do JSON parsing of our three files, transformation of the structure, and JSON generation of the resulting .vscode/settings.json and .zed/settings.json files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also just have one rust source file with everything in json!, and build it dynamically

use serde_json::{json, Value};

fn base_settings() -> Value {
    json! {{
         // common settings 
    }}
}

fn vscode_settings() -> Value {
    let vscode_specific = json! {{ anything new}} 

    let mut inner = base_settings();
    inner.as_object_mut().unwrap().extend(vscode_specific);

    json! {{
         "lsp": {
             "rust_analyzer": /* other nesting */ inner,
         }
    }}
}

fn zed_settings() -> Value {
    // ...
}

Which would also be kind of nice because it's easy to cover any config file schema or format. E.g. I would appreciate a Helix config at some point, but that uses toml (easy to convert from serde_json to toml, both are already dependencies of bootstrap).

But 🤷, that may be overkill.


/// Create a `.zed/settings.json` file for rustc development, or just print it
/// If this method should be re-called, it returns `false`.
fn create_zed_settings_maybe(config: &Config) -> io::Result<bool> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this function be deduplicated with the vscode version?

@nnethercote
Copy link
Contributor

I will defer on the review to either @bjorn3 or @tgross35, who clearly know much more about this stuff than I do :)

@tgross35
Copy link
Contributor

Should probably just see what bootstrap prefers here

r? bootstrap

@rustbot rustbot assigned Kobzol and unassigned nnethercote Jul 19, 2024
@tgross35 tgross35 removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 19, 2024
@onur-ozkan
Copy link
Member

Can Zed work with the VsCode configuration file somehow (like neovim)?

FWIW #120611 would solve the whole problem of maintaining IDE specific files.

Comment on lines +641 to +664
impl Step for Zed {
type Output = ();
const DEFAULT: bool = true;
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.alias("zed")
}
fn make_run(run: RunConfig<'_>) {
if run.builder.config.dry_run() {
return;
}
if let [cmd] = &run.paths[..] {
if cmd.assert_single_path().path.as_path().as_os_str() == "zed" {
run.builder.ensure(Zed);
}
}
}
fn run(self, builder: &Builder<'_>) -> Self::Output {
let config = &builder.config;
if config.dry_run() {
return;
}
while !t!(create_zed_settings_maybe(config)) {}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This makes it more readable

Suggested change
impl Step for Zed {
type Output = ();
const DEFAULT: bool = true;
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.alias("zed")
}
fn make_run(run: RunConfig<'_>) {
if run.builder.config.dry_run() {
return;
}
if let [cmd] = &run.paths[..] {
if cmd.assert_single_path().path.as_path().as_os_str() == "zed" {
run.builder.ensure(Zed);
}
}
}
fn run(self, builder: &Builder<'_>) -> Self::Output {
let config = &builder.config;
if config.dry_run() {
return;
}
while !t!(create_zed_settings_maybe(config)) {}
}
}
impl Step for Zed {
type Output = ();
const DEFAULT: bool = true;
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.alias("zed")
}
fn make_run(run: RunConfig<'_>) {
if run.builder.config.dry_run() {
return;
}
if let [cmd] = &run.paths[..] {
if cmd.assert_single_path().path.as_path().as_os_str() == "zed" {
run.builder.ensure(Zed);
}
}
}
fn run(self, builder: &Builder<'_>) -> Self::Output {
let config = &builder.config;
if config.dry_run() {
return;
}
while !t!(create_zed_settings_maybe(config)) {}
}
}

@onur-ozkan
Copy link
Member

FWIW #120611 would solve the whole problem of maintaining IDE specific files.

Seems like we will also need rust-lang/rust-analyzer#13529 for that.

@bors
Copy link
Contributor

bors commented Sep 8, 2024

☔ The latest upstream changes (presumably #130091) made this pull request unmergeable. Please resolve the merge conflicts.

@tgross35
Copy link
Contributor

Okay well we added emacs and helix sample config files, so maybe just adding the zed file is fine here too :) #130932

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants