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

Ensure Fluent messages are in alphabetical order #111858

Merged
merged 1 commit into from
May 26, 2023

Conversation

clubby789
Copy link
Contributor

Fixes #111847

This adds a tidy check to ensure Fluent messages are in alphabetical order, as well as sorting all existing messages. I think the error could be worded better, would appreciate suggestions.

Script used to sort files
import sys
import re

fn = sys.argv[1]
with open(fn, 'r') as f:
    data = f.read().split("\n")

chunks = []
cur = ""
for line in data:
    if re.match(r"^([a-zA-Z0-9_]+)\s*=\s*", line):
        chunks.append(cur)
        cur = ""
    cur += line + "\n"
chunks.append(cur)
chunks.sort()

with open(fn, 'w') as f:
    f.write(''.join(chunks).strip("\n\n") + "\n")

@clubby789 clubby789 added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label May 23, 2023
@rustbot
Copy link
Collaborator

rustbot commented May 23, 2023

r? @compiler-errors

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

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 23, 2023
@rustbot
Copy link
Collaborator

rustbot commented May 23, 2023

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

@rust-cloud-vms rust-cloud-vms bot force-pushed the fluent-alphabetical branch from 0d48ded to 08a6965 Compare May 23, 2023 01:57
@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member

Looks fine... cc @rust-lang/wg-diagnostics in case anyone else cares, but I generally agree that, since ftl files otherwise have literally no structure except for generally being chronologically ordered, they probably should be just alphabetical.

@oli-obk
Copy link
Contributor

oli-obk commented May 23, 2023

Should we stick your fmt script into x fmt, too? Then most people will never even hit that check.

@Noratrieb
Copy link
Member

tidy already has a --bless mode, so if you RIIR that script it would be easy to put into tidy

@rust-cloud-vms rust-cloud-vms bot force-pushed the fluent-alphabetical branch from 08a6965 to a6e1ef9 Compare May 23, 2023 11:48
@rust-cloud-vms rust-cloud-vms bot force-pushed the fluent-alphabetical branch from a6e1ef9 to e74a08a Compare May 25, 2023 23:43
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the fluent-alphabetical branch from e74a08a to f97fdda Compare May 25, 2023 23:49
@compiler-errors
Copy link
Member

Given that it seems like @jyn514 is happy and nobody else has spoken up, r=jyn,me when CI is green.

@bors p=1 (maybe bump it to 10 if this fails to merge)

@clubby789
Copy link
Contributor Author

@bors r=jyn514,compiler-errors

@bors
Copy link
Contributor

bors commented May 26, 2023

📌 Commit f97fdda has been approved by jyn514,compiler-errors

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 May 26, 2023
@bors
Copy link
Contributor

bors commented May 26, 2023

⌛ Testing commit f97fdda with merge c86212f...

@bors
Copy link
Contributor

bors commented May 26, 2023

☀️ Test successful - checks-actions
Approved by: jyn514,compiler-errors
Pushing c86212f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 26, 2023
@bors bors merged commit c86212f into rust-lang:master May 26, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c86212f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.1% [0.8%, 3.3%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.3% [1.3%, 1.3%] 1
Regressions ❌
(secondary)
2.3% [2.1%, 2.3%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.3% [1.3%, 1.3%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 644.952s -> 644.366s (-0.09%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

messages.ftl files are a source of merge conflicts
9 participants