-
Notifications
You must be signed in to change notification settings - Fork 257
Respect ${CARGO,RUSTUP}_HOME for tooltip relative dirs #1201
Conversation
I toyed around with absolute paths when I originally added the module paths to tooltips. The full paths could be quite large and I came to the conclusion that extra noise wasn't worth it. The most useful part would be at the bottom/end. VSCode also has a pretty limited-width tooltip and it almost always word wrapped. It looks pretty bad.
vs
The crate name and version, and path within the crate is both functional and doesn't overwhelm the user. |
/// assert_eq!("smallvec-0.6.2/lib.rs", tidy_path); | ||
/// let base_path = Path::new("/home/user/.cargo/registry/src/github.com-1ecc6299db9ec823/smallvec-0.6.2/lib.rs"); | ||
/// let tidy_path = skip_path_components(base_path, "/home/user/.cargo", 3); | ||
/// assert_eq!(tidy_path, Some(PathBuf::from("smallvec-0.6.2/lib.rs"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've always followed the assert(expected, actual)
convention that junit uses, but I guess the assert macro doesn't make that distinction.
e.g. https://junit.org/junit4/javadoc/4.12/org/junit/Assert.html#assertEquals(long,%20long)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I certainly didn't mean to force any changes! It's just that it read better for me due to symmetry with assigning above, so the variable is on the left side and checked expression is on the right (and the macro doesn't make any distinction, as you've said):
let var = compute_expr();
assert_eq!(var, val_of_expr);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbf the general consensus is assert_eq!(actual, expected)
you can see this in rust docs
// len docs
let mut v = HashSet::new();
assert_eq!(v.len(), 0);
v.insert(1);
assert_eq!(v.len(), 1);
I think most language/test apis use this way too, junit didn't but hamcrest & assertj, for example, do.
33d64e9
to
d6e2947
Compare
Yeah, I can see that now. I agree that the shortened version looks a lot easier to digest; I guess people should already know where they have their I'm wondering if it'd make sense to make it configurable and allow users to opt-in to the full path. It may sound silly but I know I'd like to know where exactly is the file the module is referred from 😅 |
lgtm |
Thanks! |
Update cargo, rls, miri Update cargo, rls, miri Added `rustc-workspace-hack` to miri so that it shares the same features for serde as other tools. cc @alexcrichton ## cargo 25 commits in 2cf1f5dda2f7ed84e94c4d32f643e0f1f15352f0..0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4 2018-12-11 03:44:04 +0000 to 2018-12-19 14:45:14 +0000 - Remove Stale bot's configuration (rust-lang/cargo#6463) - Add labels to issue templates (rust-lang/cargo#6464) - Fix new man page links. (rust-lang/cargo#6459) - Fix metabuild compile errors with --message-format=json. (rust-lang/cargo#6432) - Support alt-registry names in [patch] table. (rust-lang/cargo#6456) - Update the rustup URL (rust-lang/cargo#6455) - New man pages. (rust-lang/cargo#6405) - Reify the DepFingerprint type (rust-lang/cargo#6451) - Extract Fingerprint::new (rust-lang/cargo#6449) - Upgrade the metabuild to Rust 2018 (rust-lang/cargo#6448) - Make edition comparing code consistent (rust-lang/cargo#6450) - Document `name` and `authors` in [package] (rust-lang/cargo#6447) - Travis: only use mdbook 0.1.7. (rust-lang/cargo#6443) - Update git2-curl requirement from 0.8.1 to 0.9.0 (rust-lang/cargo#6439) - Update git2 requirement from 0.7.5 to 0.8.0 (rust-lang/cargo#6438) - Display errors when `cargo fix` fails. (rust-lang/cargo#6419) - cargo fix: fix targets with shared sources. (rust-lang/cargo#6434) - Fix panic-in-panic in tests. (rust-lang/cargo#6431) - More Rust 2018 edition cleanups (rust-lang/cargo#6422) - Cleanup some trait impls for SourceId (rust-lang/cargo#6429) - Remove a nightly check from doc tests (rust-lang/cargo#6427) - Replace CargoError with failure::Error (rust-lang/cargo#6425) - Allow testsuite warnings in dev (rust-lang/cargo#6426) - add `--dry-run` option to cargo update (rust-lang/cargo#6371) - Migrate to some Rust 2018 idioms (rust-lang/cargo#6416) ## rls 16 commits in bd5b899afb05e14d33e210ede3da241ca1ca088f..6f5e4bba7b1586fca6e0ea7724cadb5683b2f308 2018-12-10 08:53:00 +0100 to 2018-12-21 17:11:08 +0100 - Update jsonrpc-core (rust-lang/rls#1206) - Use `home_dir` from `home` crate (rust-lang/rls#1207) - Update cargo. (rust-lang/rls#1204) - Fix deprecated `trim_{left,right}` warnings (rust-lang/rls#1203) - Respect ${CARGO,RUSTUP}_HOME for tooltip relative dirs (rust-lang/rls#1201) - Separate tooltip tests that require Racer fallback (rust-lang/rls#1200) - tests: Don't generate tooltip results in tests/fixtures (rust-lang/rls#1199) - Overhaul fixture handling in tests (rust-lang/rls#1190) - Don't return symbols with empty names (rust-lang/rls#1193) - Don't check AppVeyor CI status for bors - Properly infer full_docs (rust-lang/rls#1192) - Update cargo (rust-lang/rls#1191) - Improve hover test_tooltip tests (rust-lang/rls#1175) - Fix unused warnings (rust-lang/rls#1185) - Workaround rust-lang/rls#703 to prevent obscure failures due to sccache. (rust-lang/rls#1177) - Disable travis cache (rust-lang/rls#1182) ## miri 14 commits in bccadeb..6c2fc6d 2018-12-08 11:07:22 +0100 to 2018-12-26 14:28:25 +0100 - use memory::check_bounds_ptr for offset check (rust-lang/miri#589) - Fix comparing function pointers (rust-lang/miri#587) - fix for infallible allocation (rust-lang/miri#586) - fix test for latest nightly (rust-lang/miri#585) - Treat ref-to-raw cast like a reborrow: do a special kind of retag (rust-lang/miri#572) - Test cargo-miri on Windows (rust-lang/miri#578) - Cargo miri tweaks and test that we can exclude tests (rust-lang/miri#580) - Fix cargo miri test (rust-lang/miri#550) - fix for latest nightly (rust-lang/miri#574) - Add rustc-workspace-hack. (rust-lang/miri#575) - use RUSTC_WRAPPER for the cargo hook (rust-lang/miri#573) - do not auto-detect the targets in the sysroot, instead specify target manually through env var (rust-lang/miri#570) - Cleanup: Avoid repeating signatures, get rid of to_bytes hack (rust-lang/miri#568) - Support building and running with full MIR on foreign architectures, drop support for missing MIR (rust-lang/miri#566)
In principle, I'd say I prefer absolute paths for references to outside of the project root (or at least relative to
${CARGO,RUSTUP_HOME}
in this case) but it's what we test against now and didn't want to cause any change in behaviour now.I'd appreciate more eyes looking at this: @ehuss @alexheretic @aloucks
Should fix Rust CI failure rust-lang/rust#56924 (there, we map
.cargo
to/cargo
and the old logic only remapped paths starting from.cargo
).