Skip to content

Add modulo and mod as doc aliases for rem_euclid. #114977

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

Merged
merged 1 commit into from
Aug 19, 2023
Merged

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Aug 18, 2023

When I was learning Rust I looked for “a modulo function” and couldn’t find one, so thought I had to write my own; it wasn't at all obvious that a function with “rem” in the name was the function I wanted. Hopefully this will save the next learner from that.

However, it does have the disadvantage that the top results in rustdoc for “mod” are now these aliases instead of the Rust keyword, which probably isn't ideal.

When I was learning Rust I looked for “a modulo function” and couldn’t
find one, so thought I had to write my own; it wasn't at all obvious
that a function with “rem” in the name was the function I wanted.
Hopefully this will save the next learner from that.

However, it does have the disadvantage that the top results in rustdoc
for “mod” are now these aliases instead of the Rust keyword, which
probably isn't ideal.
@rustbot
Copy link
Collaborator

rustbot commented Aug 18, 2023

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

@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 Aug 18, 2023
@thomcc
Copy link
Member

thomcc commented Aug 18, 2023

Needs an ACP Oh, doc aliases, sorry.

Uh, hm.

@thomcc
Copy link
Member

thomcc commented Aug 18, 2023

I think this is fine for modulo, but perhaps not for mod (for the reason you mention).

@kpreid
Copy link
Contributor Author

kpreid commented Aug 18, 2023

Let me present further argument for keeping “mod” in particular:

  1. In most (not all) programming languages which have it as a function, the name is some version of “mod”, not “modulo”, and rustdoc won't show the alias unless you type the full name (no substring completions), so including only “modulo” as an alias will not help the people who type “mod”.
  2. The harm is low because rustdoc search is primarily a place you go to for items — people looking for syntax are probably as likely to consult other documentation first, and mod is hard to miss such that someone would be in the “look up this unfamiliar thing” phase rather than “learning the fundamentals of Rust from some resource”.
    • It can be mitigated by a tweak to rustdoc's ordering such that exact match non-aliases appear above aliases.

However, I'm happy to adjust this PR to only include “modulo” if that is unconvincing.

@thomcc
Copy link
Member

thomcc commented Aug 18, 2023

Hm, I think this is fine. This is probably my pick for the worst-named function in the stdlib anyway.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 18, 2023

📌 Commit 2c21635 has been approved by thomcc

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 Aug 18, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 19, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#114953 (Add myself back to review rotation)
 - rust-lang#114958 (`ignore-cross-compile` on `optimization-remarks-dir-pgo` test)
 - rust-lang#114971 (Add doc aliases for trigonometry and other f32,f64 methods.)
 - rust-lang#114972 (Add a test to check that inline const is in required_consts)
 - rust-lang#114977 (Add `modulo` and `mod` as doc aliases for `rem_euclid`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8e0a8cd into rust-lang:master Aug 19, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 19, 2023
@kpreid kpreid deleted the modulo branch August 29, 2023 19:13
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.

4 participants