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

improve btree_cursors functions documentation #120936

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Conversation

ripytide
Copy link
Contributor

@ripytide ripytide commented Feb 11, 2024

As suggested by @Amanieu (and others) in #107540 (#107540 (comment))

Improvements:

  • Document exact behavior of {upper/lower}_bound{,_mut} with each of the three Bound types using unambigous words {greatest,greater,smallest,smaller,before,after}.
  • Added another doc-example for the Bound::Unbounded for each of the methods
  • Changed doc-example to use From<[T; N]> rather than lots of insert()s which requires a mutable map which clutters the example when mut may not be required for the method (such as for {upper,lower}_bound.
  • Removed # Panics section from insert_{before,after} methods since they were changed to return an error instead a while ago.
  • Reworded some phrases to be more consistent with the more regular BTreeMap methods such as calling entries "key-value" rather than "element"s.

@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2024

r? @joshtriplett

rustbot has assigned @joshtriplett.
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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 11, 2024
@ripytide
Copy link
Contributor Author

r? @Amanieu

@rust-log-analyzer

This comment has been minimized.

@momvart
Copy link
Contributor

momvart commented Feb 11, 2024

I think less/least is more common than smaller/smallest as the opposite of greater/greatest.

@ripytide
Copy link
Contributor Author

ripytide commented Feb 11, 2024

good point, switching them now

@ripytide
Copy link
Contributor Author

ripytide commented Feb 11, 2024

Swapping Smallest for Least doesn't sound right: Returns a [Cursor] pointing at the gap before the least key greater than the given bound.. I think Smaller is most common here instead of least at which point you are already using the word smaller so it is more consistent keep using smaller over less?

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@momvart
Copy link
Contributor

momvart commented Feb 11, 2024

Swapping Smallest for Least doesn't sound right

Yeah it looks a bit weird. I found "smaller than equal" a bit unfamiliar to stand for <=. But anyway, it probably works well in these sentences.

@Amanieu
Copy link
Member

Amanieu commented Feb 11, 2024

Thanks! This is much more understandable.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 11, 2024

📌 Commit f34d9da has been approved by Amanieu

It is now in the queue for this repository.

@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 Feb 11, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 12, 2024
improve `btree_cursors` functions documentation

As suggested by `@Amanieu` (and others) in rust-lang#107540 (rust-lang#107540 (comment))

Improvements:
- Document exact behavior of `{upper/lower}_bound{,_mut}` with each of the three `Bound` types using unambigous words `{greatest,greater,smallest,smaller,before,after}`.
- Added another doc-example for the `Bound::Unbounded` for each of the methods
- Changed doc-example to use From<[T; N]> rather than lots of `insert()`s which requires a mutable map which clutters the example when `mut` may not be required for the method (such as for `{upper,lower}_bound`.
- Removed `# Panics` section from `insert_{before,after}` methods since they were changed to return an error instead a while ago.
- Reworded some phrases to be more consistent with the more regular `BTreeMap` methods such as calling entries "key-value" rather than "element"s.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#120765 (Reorder diagnostics API)
 - rust-lang#120833 (More internal emit diagnostics cleanups)
 - rust-lang#120899 (Gracefully handle non-WF alias in `assemble_alias_bound_candidates_recur`)
 - rust-lang#120917 (Remove a bunch of dead parameters in functions)
 - rust-lang#120928 (Add test for recently fixed issue)
 - rust-lang#120933 (check_consts: fix duplicate errors, make importance consistent)
 - rust-lang#120936 (improve `btree_cursors` functions documentation)
 - rust-lang#120944 (Check that the ABI of the instance we are inlining is correct)
 - rust-lang#120956 (Clean inlined type alias with correct param-env)
 - rust-lang#120962 (Add myself to library/std review)
 - rust-lang#120972 (fix ICE for deref coercions with type errors)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8305686 into rust-lang:master Feb 12, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2024
Rollup merge of rust-lang#120936 - ripytide:master, r=Amanieu

improve `btree_cursors` functions documentation

As suggested by ``@Amanieu`` (and others) in rust-lang#107540 (rust-lang#107540 (comment))

Improvements:
- Document exact behavior of `{upper/lower}_bound{,_mut}` with each of the three `Bound` types using unambigous words `{greatest,greater,smallest,smaller,before,after}`.
- Added another doc-example for the `Bound::Unbounded` for each of the methods
- Changed doc-example to use From<[T; N]> rather than lots of `insert()`s which requires a mutable map which clutters the example when `mut` may not be required for the method (such as for `{upper,lower}_bound`.
- Removed `# Panics` section from `insert_{before,after}` methods since they were changed to return an error instead a while ago.
- Reworded some phrases to be more consistent with the more regular `BTreeMap` methods such as calling entries "key-value" rather than "element"s.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 26, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#120765 (Reorder diagnostics API)
 - rust-lang#120833 (More internal emit diagnostics cleanups)
 - rust-lang#120899 (Gracefully handle non-WF alias in `assemble_alias_bound_candidates_recur`)
 - rust-lang#120917 (Remove a bunch of dead parameters in functions)
 - rust-lang#120928 (Add test for recently fixed issue)
 - rust-lang#120933 (check_consts: fix duplicate errors, make importance consistent)
 - rust-lang#120936 (improve `btree_cursors` functions documentation)
 - rust-lang#120944 (Check that the ABI of the instance we are inlining is correct)
 - rust-lang#120956 (Clean inlined type alias with correct param-env)
 - rust-lang#120962 (Add myself to library/std review)
 - rust-lang#120972 (fix ICE for deref coercions with type errors)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants