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

feat: always show constructor inlay hints, add option to disable #10761

Merged
merged 2 commits into from Nov 13, 2021

Conversation

jhgg
Copy link
Contributor

@jhgg jhgg commented Nov 13, 2021

This PR adds a config to disable the functionality in #10441 - and makes that functionality disabled by default.

I actually really like inlay hints always showing up, and it helps a lot as a teaching tool too, and also it's quite reassuring to know that r-a indeed understands the code I've written. This PR adds the option rust-analyzer.inlayHints.hideNamedConstructorHints (i can also invert this to be rust-analyzer.inlayHints.showNamedConstructorHints, just let me know.)

Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

Great, wanted to do this as well but I somehow forgot about this, as I also prefer always showing the hints.

Not sure about whether we want to go with show or hide as the name. show would fit with the rest of things as usually we go for *enable, but at the same time this is a specific thing that removes from the general inlayhints feature so I think hide does fit here.

What I would change though is that the default value should show named constructors, as it probably would be rather confusing for newcomers, wondering why some inlay hints are missing.

@jhgg
Copy link
Contributor Author

jhgg commented Nov 13, 2021

I'm totally cool with changing the default to always show inlay hints. I think it is confusing if inlay hints sometimes show and do not show. But I also didn't want to essentially erase the functionality in 10441 by making it the non-default (most people don't really change config.)

But if you agree that always showing inlay hints is a more sensible default (and I concur with this), I'll flip the default.

@Veykril
Copy link
Member

Veykril commented Nov 13, 2021

While we do want to make everthing as much as discoverable we should still keep sensible defaults in mind imo, and this one is one of them. I feel like people would not easily realize that a type hint is missing because of a constructor(in fact even though I implemented that feature I at times wondered why the type hint was missing for a moment), I personally think this feature is more confusing than helpful as well, though I can see why someone would maybe want it.

@jhgg
Copy link
Contributor Author

jhgg commented Nov 13, 2021

Done!

@Veykril
Copy link
Member

Veykril commented Nov 13, 2021

Thanks!

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 13, 2021

@bors bors bot merged commit 766b52b into rust-lang:master Nov 13, 2021
@jhgg jhgg deleted the inlay-hints-constructor-option branch November 14, 2021 01:16
@lnicola lnicola changed the title inlay hints: add the option to always show constructor inlay hints feat: always show constructor inlay hints, add option to disable Nov 14, 2021
bors added a commit that referenced this pull request May 20, 2022
feat: hide type inlay hints for initializations of closures

![hide_closure_initialization](https://user-images.githubusercontent.com/12008103/168470158-6cb77b18-068e-4431-a8b5-e2b22d50d263.gif)

This PR adds an option to hide the inlay hints for `let IDENT_PAT = CLOSURE_EXPR;`, which is a somewhat common coding pattern. Currently the inlay hints for the assigned variable and the closure expression itself are both displayed, making it rather repetitive.

In order to be consistent with closure return type hints, only closures with block bodies will be hid by this option.

Personally I'd feel comfortable making it always enabled (or at least when closure return type hints are enabled), but considering the precedent set in #10761, I introduced an off-by-default option for this.

changelog feature: option to hide type inlay hints for initializations of closures
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
perf: improve write_fmt to handle simple strings

In case format string has no arguments, simplify its implementation with a direct call to `output.write_str(value)`. This builds on `@dtolnay` original [suggestion](serde-rs/serde#2697 (comment)). This does not change any expectations because the original `fn write()` implementation calls `write_str` for parts of the format string.

```rust
write!(f, "text")  ->  f.write_str("text")
```

```diff
 /// [`write!`]: crate::write!
+#[inline]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result {
+    if let Some(s) = args.as_str() { output.write_str(s) } else { write_internal(output, args) }
+}
+
+/// Actual implementation of the [`write`], but without the simple string optimization.
+fn write_internal(output: &mut dyn Write, args: Arguments<'_>) -> Result {
     let mut formatter = Formatter::new(output);
     let mut idx = 0;
```

* Hopefully it will improve the simple case for the rust-lang/rust#99012
* Another related (original?) issues rust-lang#10761
* Previous similar attempt to fix it by by `@Kobzol` #100700

CC: `@m-ou-se` as probably the biggest expert in everything `format!`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
perf: improve write_fmt to handle simple strings

In case format string has no arguments, simplify its implementation with a direct call to `output.write_str(value)`. This builds on `@dtolnay` original [suggestion](serde-rs/serde#2697 (comment)). This does not change any expectations because the original `fn write()` implementation calls `write_str` for parts of the format string.

```rust
write!(f, "text")  ->  f.write_str("text")
```

```diff
 /// [`write!`]: crate::write!
+#[inline]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result {
+    if let Some(s) = args.as_str() { output.write_str(s) } else { write_internal(output, args) }
+}
+
+/// Actual implementation of the [`write`], but without the simple string optimization.
+fn write_internal(output: &mut dyn Write, args: Arguments<'_>) -> Result {
     let mut formatter = Formatter::new(output);
     let mut idx = 0;
```

* Hopefully it will improve the simple case for the rust-lang/rust#99012
* Another related (original?) issues rust-lang#10761
* Previous similar attempt to fix it by by `@Kobzol` #100700

CC: `@m-ou-se` as probably the biggest expert in everything `format!`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants