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

Ask folks to use the Oxford comma in diagnostics #2093

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

onkoe
Copy link

@onkoe onkoe commented Oct 8, 2024

Hey there! In Rust, it seems that most user-facing types of content prefer unambiguous grammar. However, there currently isn't any instruction to use the Oxford comma when writing lists of items within diagnostics.

I added that to avoid any ambiguity, such as that which I fixed in rust-lang/rust#131038

Please let me know if any changes are necessary! :)

we encountered this in rust-lang/rust#131038

kinda felt like an unspoken rule so here's a more concrete
clarification.
@jieyouxu jieyouxu added the S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content label Oct 13, 2024
Comment on lines +159 to +160
- Use the [Oxford comma](https://en.wikipedia.org/wiki/Serial_comma) when
writing lists of items.
Copy link
Member

@jieyouxu jieyouxu Nov 4, 2024

Choose a reason for hiding this comment

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

Nit: I don't think we want to make this advice because just like what the wikipedia entry says

The serial comma can serve to avoid ambiguity in specific contexts, though its employment may also generate ambiguity under certain circumstances.

I think it's context-dependent and is best left to the reviewer/author's judgement for whether or not to deploy oxford commas.

Copy link
Member

Choose a reason for hiding this comment

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

I have rarely seen it add ambiguity, and have far more seen it's lack add ambiguity

Copy link
Author

@onkoe onkoe Nov 4, 2024

Choose a reason for hiding this comment

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

Thank you for the review! It's a good call to think a bit before writing diagnostics. However, I intentionally qualified it with "when listing items".

The ambiguities stemming from the extra comma are pretty rare. When they do appear, it's typically because the author was doing more than listing items.

With that in mind, do you think softer wording would be more effective? My goal was to avoid ambiguity in the diagnostic formatting rules.

Copy link
Member

@jieyouxu jieyouxu Nov 4, 2024

Choose a reason for hiding this comment

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

I don't have too strong of an opinion on this, so if tshepang is in favor then we can merge and see if this advice turns out to not always apply in practice (I suspect rarely). We can always adjust this advice from more experience.

@jieyouxu jieyouxu added A-style Area: style A-diagnostics Area: diagnostics WG-diagnostics Working group: diagnostics T-compiler Relevant to compiler team labels Nov 4, 2024
@tshepang tshepang merged commit 36f61df into rust-lang:master Nov 5, 2024
1 check passed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 18, 2024
Update books

## rust-lang/edition-guide

2 commits in 2d482e203eb6d6e353814cf1415c5f94e590b9e0..915f9b319c2823f310430ecdecd86264a7870d7e
2024-11-06 07:23:07 UTC to 2024-11-05 09:03:41 UTC

- 2021: Update for raw lifetimes (rust-lang/edition-guide#330)
- CI: Switch to merge queue (rust-lang/edition-guide#333)

## rust-lang/nomicon

1 commits in 456b904f791751892b01282fd2757904993c4c26..eac89a3cbe6c4714e5029ae8b5a1c556fd4e8c42
2024-11-16 14:05:28 UTC to 2024-11-16 14:05:28 UTC

- Fix typo in what-unsafe-does (rust-lang/nomicon#469)

## rust-lang/reference

10 commits in da0f6dad767670da0e8cd5af8a7090db3272f626..41ccb0e6478305401dad92e8fd3d04a4304edb4c
2024-11-15 21:45:16 UTC to 2024-11-05 21:46:30 UTC

- Add identifiers to attributes.md and its subchapters (rust-lang/reference#1560)
- Fix 2 typos (rust-lang/reference#1674)
- Add examples to clarify the casting rules (rust-lang/reference#686)
- Explaining how to link mixed C/Rust binaries. (rust-lang/reference#838)
- Add Arm64EC to inline-assembly documentation (rust-lang/reference#1653)
- Add three more WebAssembly features to the list of accepted ones (rust-lang/reference#1638)
- Add s390x to inline-assembly documentation (rust-lang/reference#1643)
- trait object constraint correction (rust-lang/reference#1670)
- Update some "default" representation references (rust-lang/reference#1667)
- Update lifetimes for pre-expansion validation (rust-lang/reference#1668)

## rust-lang/rustc-dev-guide

12 commits in 6a5accdaf10255882b1e6c59dfe5f1c79ac95484..b679e71c2d66c6fe13e06b99ac61773b866213f0
2024-11-18 08:18:15 UTC to 2024-11-05 07:22:17 UTC

- Mention `RUSTC_BOOTSTRAP` for misc testing (rust-lang/rustc-dev-guide#2136)
- Document how to acquire `cdb.exe` (rust-lang/rustc-dev-guide#2137)
- Document `max-llvm-major-version` directive (rust-lang/rustc-dev-guide#2129)
- Document `exact-llvm-major-version` directive (rust-lang/rustc-dev-guide#2135)
- Note Rustfmt for separate rust-analyzer directory (rust-lang/rustc-dev-guide#2134)
- still accurate (rust-lang/rustc-dev-guide#2133)
- typo (rust-lang/rustc-dev-guide#2132)
- add valid date-check marker (rust-lang/rustc-dev-guide#2131)
- Update parallel-rustc.md (rust-lang/rustc-dev-guide#1926)
- Rename `{ignore,only}-debug` -> `{ignore,needs}-{rustc,std}-debug-assertions` (rust-lang/rustc-dev-guide#2101)
- update const stability docs (rust-lang/rustc-dev-guide#2111)
- Ask folks to use the Oxford comma in diagnostics (rust-lang/rustc-dev-guide#2093)
fmease added a commit to fmease/rust that referenced this pull request Nov 19, 2024
Update books

## rust-lang/edition-guide

2 commits in 2d482e203eb6d6e353814cf1415c5f94e590b9e0..915f9b319c2823f310430ecdecd86264a7870d7e
2024-11-06 07:23:07 UTC to 2024-11-05 09:03:41 UTC

- 2021: Update for raw lifetimes (rust-lang/edition-guide#330)
- CI: Switch to merge queue (rust-lang/edition-guide#333)

## rust-lang/nomicon

1 commits in 456b904f791751892b01282fd2757904993c4c26..eac89a3cbe6c4714e5029ae8b5a1c556fd4e8c42
2024-11-16 14:05:28 UTC to 2024-11-16 14:05:28 UTC

- Fix typo in what-unsafe-does (rust-lang/nomicon#469)

## rust-lang/reference

10 commits in da0f6dad767670da0e8cd5af8a7090db3272f626..41ccb0e6478305401dad92e8fd3d04a4304edb4c
2024-11-15 21:45:16 UTC to 2024-11-05 21:46:30 UTC

- Add identifiers to attributes.md and its subchapters (rust-lang/reference#1560)
- Fix 2 typos (rust-lang/reference#1674)
- Add examples to clarify the casting rules (rust-lang/reference#686)
- Explaining how to link mixed C/Rust binaries. (rust-lang/reference#838)
- Add Arm64EC to inline-assembly documentation (rust-lang/reference#1653)
- Add three more WebAssembly features to the list of accepted ones (rust-lang/reference#1638)
- Add s390x to inline-assembly documentation (rust-lang/reference#1643)
- trait object constraint correction (rust-lang/reference#1670)
- Update some "default" representation references (rust-lang/reference#1667)
- Update lifetimes for pre-expansion validation (rust-lang/reference#1668)

## rust-lang/rustc-dev-guide

12 commits in 6a5accdaf10255882b1e6c59dfe5f1c79ac95484..b679e71c2d66c6fe13e06b99ac61773b866213f0
2024-11-18 08:18:15 UTC to 2024-11-05 07:22:17 UTC

- Mention `RUSTC_BOOTSTRAP` for misc testing (rust-lang/rustc-dev-guide#2136)
- Document how to acquire `cdb.exe` (rust-lang/rustc-dev-guide#2137)
- Document `max-llvm-major-version` directive (rust-lang/rustc-dev-guide#2129)
- Document `exact-llvm-major-version` directive (rust-lang/rustc-dev-guide#2135)
- Note Rustfmt for separate rust-analyzer directory (rust-lang/rustc-dev-guide#2134)
- still accurate (rust-lang/rustc-dev-guide#2133)
- typo (rust-lang/rustc-dev-guide#2132)
- add valid date-check marker (rust-lang/rustc-dev-guide#2131)
- Update parallel-rustc.md (rust-lang/rustc-dev-guide#1926)
- Rename `{ignore,only}-debug` -> `{ignore,needs}-{rustc,std}-debug-assertions` (rust-lang/rustc-dev-guide#2101)
- update const stability docs (rust-lang/rustc-dev-guide#2111)
- Ask folks to use the Oxford comma in diagnostics (rust-lang/rustc-dev-guide#2093)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2024
Rollup merge of rust-lang#133181 - rustbot:docs-update, r=ehuss

Update books

## rust-lang/edition-guide

2 commits in 2d482e203eb6d6e353814cf1415c5f94e590b9e0..915f9b319c2823f310430ecdecd86264a7870d7e
2024-11-06 07:23:07 UTC to 2024-11-05 09:03:41 UTC

- 2021: Update for raw lifetimes (rust-lang/edition-guide#330)
- CI: Switch to merge queue (rust-lang/edition-guide#333)

## rust-lang/nomicon

1 commits in 456b904f791751892b01282fd2757904993c4c26..eac89a3cbe6c4714e5029ae8b5a1c556fd4e8c42
2024-11-16 14:05:28 UTC to 2024-11-16 14:05:28 UTC

- Fix typo in what-unsafe-does (rust-lang/nomicon#469)

## rust-lang/reference

10 commits in da0f6dad767670da0e8cd5af8a7090db3272f626..41ccb0e6478305401dad92e8fd3d04a4304edb4c
2024-11-15 21:45:16 UTC to 2024-11-05 21:46:30 UTC

- Add identifiers to attributes.md and its subchapters (rust-lang/reference#1560)
- Fix 2 typos (rust-lang/reference#1674)
- Add examples to clarify the casting rules (rust-lang/reference#686)
- Explaining how to link mixed C/Rust binaries. (rust-lang/reference#838)
- Add Arm64EC to inline-assembly documentation (rust-lang/reference#1653)
- Add three more WebAssembly features to the list of accepted ones (rust-lang/reference#1638)
- Add s390x to inline-assembly documentation (rust-lang/reference#1643)
- trait object constraint correction (rust-lang/reference#1670)
- Update some "default" representation references (rust-lang/reference#1667)
- Update lifetimes for pre-expansion validation (rust-lang/reference#1668)

## rust-lang/rustc-dev-guide

12 commits in 6a5accdaf10255882b1e6c59dfe5f1c79ac95484..b679e71c2d66c6fe13e06b99ac61773b866213f0
2024-11-18 08:18:15 UTC to 2024-11-05 07:22:17 UTC

- Mention `RUSTC_BOOTSTRAP` for misc testing (rust-lang/rustc-dev-guide#2136)
- Document how to acquire `cdb.exe` (rust-lang/rustc-dev-guide#2137)
- Document `max-llvm-major-version` directive (rust-lang/rustc-dev-guide#2129)
- Document `exact-llvm-major-version` directive (rust-lang/rustc-dev-guide#2135)
- Note Rustfmt for separate rust-analyzer directory (rust-lang/rustc-dev-guide#2134)
- still accurate (rust-lang/rustc-dev-guide#2133)
- typo (rust-lang/rustc-dev-guide#2132)
- add valid date-check marker (rust-lang/rustc-dev-guide#2131)
- Update parallel-rustc.md (rust-lang/rustc-dev-guide#1926)
- Rename `{ignore,only}-debug` -> `{ignore,needs}-{rustc,std}-debug-assertions` (rust-lang/rustc-dev-guide#2101)
- update const stability docs (rust-lang/rustc-dev-guide#2111)
- Ask folks to use the Oxford comma in diagnostics (rust-lang/rustc-dev-guide#2093)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: diagnostics A-style Area: style S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content T-compiler Relevant to compiler team WG-diagnostics Working group: diagnostics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants