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

Cleanup: Rename ModSep to PathSep #123462

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

fmease
Copy link
Member

@fmease fmease commented Apr 4, 2024

:: is usually referred to as the path separator (citation needed).

The existing name ModSep for module separator is a bit misleading since it in fact separates the segments of arbitrary path segments, not only ones resolving to modules. Let me just give a shout-out to associated items (T::Assoc, <Ty as Trait>::function) and enum variants (Option::None).

Motivation: Reduce friction for new contributors, prevent potential confusion.

cc @petrochenkov
r? nnethercote or compiler

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 4, 2024
@fmease fmease added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Apr 4, 2024
@rust-log-analyzer

This comment has been minimized.

@Noratrieb
Copy link
Member

citation needed

you could cite this https://www.baeldung.com/java-file-vs-file-path-separator

no but uh I think that rename makes a lot of sense

@rustbot
Copy link
Collaborator

rustbot commented Apr 4, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@ChrisDenton
Copy link
Member

ChrisDenton commented Apr 4, 2024

Aside: I wish we had better terminology to distinguish between filesystem paths and other types of path.

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Apr 4, 2024

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@fmease
Copy link
Member Author

fmease commented Apr 4, 2024

/me keep forgetting about all the tools we have 🤦

@nnethercote
Copy link
Contributor

Sorry for the slow response, I have been on vacation.

The tokens chapter of the Reference calls it "path separator", which is good enough for me.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 15, 2024

📌 Commit 3cbc9e9 has been approved by nnethercote

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 Apr 15, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#123016 (Remove `TypeVariableOriginKind` and `ConstVariableOriginKind`)
 - rust-lang#123462 (Cleanup: Rename `ModSep` to `PathSep`)
 - rust-lang#123603 (Don't even parse an intrinsic unless the feature gate is enabled)
 - rust-lang#123926 (Fix pretty HIR for anon consts in diagnostics)
 - rust-lang#123973 (crashes: readme: add reminder to add Fixes #abcde to prs to automatically close issues.)
 - rust-lang#123984 (sanitizers: Add rustc_sanitizers to triagebot.toml)
 - rust-lang#123989 (Just use `type_dependent_def_id` to figure out what the method is for an expr)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c566599 into rust-lang:master Apr 16, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2024
Rollup merge of rust-lang#123462 - fmease:rn-mod-sep-to-path-sep, r=nnethercote

Cleanup: Rename `ModSep` to `PathSep`

`::` is usually referred to as the *path separator* (citation needed).

The existing name `ModSep` for *module separator* is a bit misleading since it in fact separates the segments of arbitrary path segments, not only ones resolving to modules. Let me just give a shout-out to associated items (`T::Assoc`, `<Ty as Trait>::function`) and enum variants (`Option::None`).

Motivation: Reduce friction for new contributors, prevent potential confusion.

cc `@petrochenkov`
r? nnethercote or compiler
@fmease fmease deleted the rn-mod-sep-to-path-sep branch April 16, 2024 07:21
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 18, 2024
…nethercote

Cleanup: Rename `ModSep` to `PathSep`

`::` is usually referred to as the *path separator* (citation needed).

The existing name `ModSep` for *module separator* is a bit misleading since it in fact separates the segments of arbitrary path segments, not only ones resolving to modules. Let me just give a shout-out to associated items (`T::Assoc`, `<Ty as Trait>::function`) and enum variants (`Option::None`).

Motivation: Reduce friction for new contributors, prevent potential confusion.

cc `@petrochenkov`
r? nnethercote or compiler
calebcartwright pushed a commit to calebcartwright/rust that referenced this pull request Jun 22, 2024
…nethercote

Cleanup: Rename `ModSep` to `PathSep`

`::` is usually referred to as the *path separator* (citation needed).

The existing name `ModSep` for *module separator* is a bit misleading since it in fact separates the segments of arbitrary path segments, not only ones resolving to modules. Let me just give a shout-out to associated items (`T::Assoc`, `<Ty as Trait>::function`) and enum variants (`Option::None`).

Motivation: Reduce friction for new contributors, prevent potential confusion.

cc `@petrochenkov`
r? nnethercote or compiler
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-compiler Relevant to the compiler 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