Skip to content

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Sep 4, 2020

Sublime Text folds code based on indentation. It maybe an unnecessary change, but does it look nicer after that ?

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 4, 2020
@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 4, 2020
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

r=me with nit fixed

@jyn514
Copy link
Member

jyn514 commented Sep 4, 2020

You also need to run x.py test src/test/ui --bless.

@tesuji tesuji force-pushed the indent-note branch 2 times, most recently from ae5b7cc to 7347aa9 Compare September 4, 2020 03:42
note = "you can use `.chars().nth()` or `.bytes().nth()`
see chapter in The Book <https://doc.rust-lang.org/book/ch08-02-strings.html#indexing-into-strings>"
note = "you can use `.chars().nth()` or `.bytes().nth()`\n\
For more information, see chapter 8 in The Book: \
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 is changing it to visual indent (following the position of the ""), but it should be block-indented instead, per https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/principles.md#overarching-guidelines

So something like

    on(
        all(any(T = "str", T = "&str", T = "std::string::String"), _Self = "{integer}"),
        note = "you can use `.chars().nth()` or `.bytes().nth()`\n\
            For more information, see chapter 8 in The Book: \

(Note how the all is indented 4, not 3 as it would be if it were following on()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That case is more like:

// Block indent
a_function_call(foo,
    bar,
);

@jyn514
Copy link
Member

jyn514 commented Sep 4, 2020

LGTM, r? @scottmcm #76309 (comment)

@rust-highfive rust-highfive assigned scottmcm and unassigned jyn514 Sep 4, 2020
@scottmcm
Copy link
Member

scottmcm commented Sep 4, 2020

Looks like there are some more test failures in PR here, @lzutao?

@scottmcm scottmcm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 4, 2020

Needs you to run --bless again.

---- [ui] ui/str/str-mut-idx.rs stdout ----
diff of stderr:

39	   |
40	   = help: the trait `SliceIndex<str>` is not implemented for `{integer}`
41	   = note: you can use `.chars().nth()` or `.bytes().nth()`
-	           For more information, see chapter 8 in The Book: <https://doc.rust-lang.org/book/ch08-02-strings.html#indexing-into-strings>
+	           for more information, see chapter 8 in The Book: <https://doc.rust-lang.org/book/ch08-02-strings.html#indexing-into-strings>
43	
44	error[E0277]: the type `str` cannot be indexed by `{integer}`
45	  --> $DIR/str-mut-idx.rs:11:25

49	   |
50	   = help: the trait `SliceIndex<str>` is not implemented for `{integer}`
51	   = note: you can use `.chars().nth()` or `.bytes().nth()`
-	           For more information, see chapter 8 in The Book: <https://doc.rust-lang.org/book/ch08-02-strings.html#indexing-into-strings>
+	           for more information, see chapter 8 in The Book: <https://doc.rust-lang.org/book/ch08-02-strings.html#indexing-into-strings>
53	
54	error[E0277]: the type `str` cannot be indexed by `char`
55	  --> $DIR/str-mut-idx.rs:13:5

Co-authored-by: Joshua Nelson <joshua@yottadb.com>
@tesuji
Copy link
Contributor Author

tesuji commented Sep 5, 2020

Thanks, blessed the tests.

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 5, 2020

@bors r+ rollup

@scottmcm if you want to fix the block indentation feel free to make a follow-up PR ;) but I think this is an improvement over the current code

@bors
Copy link
Collaborator

bors commented Sep 5, 2020

📌 Commit dfd219d has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2020
@scottmcm
Copy link
Member

scottmcm commented Sep 5, 2020

@jyn514 All good -- I looked at the other places that attribute is used and there's at least two different indentation styles in use already 🤷‍♀️

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 6, 2020
Indent a note to make folding work nicer

Sublime Text folds code based on indentation. It maybe an unnecessary change, but does it look nicer after that ?
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2020
Rollup of 18 pull requests

Successful merges:

 - rust-lang#76273 (Move some Vec UI tests into alloc unit tests)
 - rust-lang#76274 (Allow try blocks as the argument to return expressions)
 - rust-lang#76287 (Remove an unnecessary allowed lint)
 - rust-lang#76293 (Implementation of incompatible features error)
 - rust-lang#76299 (Make `Ipv4Addr` and `Ipv6Addr` const tests unit tests under `library`)
 - rust-lang#76302 (Address review comments on `Peekable::next_if`)
 - rust-lang#76303 (Link to `#capacity-and-reallocation` when using with_capacity)
 - rust-lang#76305 (Move various ui const tests to `library`)
 - rust-lang#76309 (Indent a note to make folding work nicer)
 - rust-lang#76312 (time.rs: Make spelling of "Darwin" consistent)
 - rust-lang#76318 (Use ops::ControlFlow in rustc_data_structures::graph::iterate)
 - rust-lang#76324 (Move Vec slice UI tests in library)
 - rust-lang#76338 (add some intra-doc links to `Iterator`)
 - rust-lang#76340 (Remove unused duplicated `trivial_dropck_outlives`)
 - rust-lang#76344 (Improve docs for `std::env::args()`)
 - rust-lang#76346 (Docs: nlink example typo)
 - rust-lang#76358 (Minor grammar fix in doc comment for soft-deprecated methods)
 - rust-lang#76364 (Disable atomics on avr target.)

Failed merges:

 - rust-lang#76304 (Make delegation methods of `std::net::IpAddr` unstably const)

r? @ghost
@bors bors merged commit 8ff13f4 into rust-lang:master Sep 7, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 7, 2020
@tesuji tesuji deleted the indent-note branch September 7, 2020 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants