Skip to content

[FIX] Sort the capitalization and profanity lists with the C comparator, in the active locale - #2460

Open
Deepak-negi11 wants to merge 1 commit into
CCExtractor:masterfrom
Deepak-negi11:fix/word-list-sort-locale
Open

[FIX] Sort the capitalization and profanity lists with the C comparator, in the active locale#2460
Deepak-negi11 wants to merge 1 commit into
CCExtractor:masterfrom
Deepak-negi11:fix/word-list-sort-locale

Conversation

@Deepak-negi11

@Deepak-negi11 Deepak-negi11 commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

In raising this pull request, I confirm the following (please check boxes):

Reason for this PR:

  • This PR adds new functionality.
  • This PR fixes a bug that I have personally experienced or that a real user has reported and for which a sample exists.
  • This PR is porting code from C to Rust.

Sanity check:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • If the PR adds new functionality, I've added it to the changelog. If it's just a bug fix, I have NOT added it to the changelog.
  • I am NOT adding new C code unless it's to fix an existing, reproducible bug.

Repro instructions:

Samples are attached: ankara.scc (one caption, "ANKARA ON HALLOWEEN.") and cities.txt (one line: Ankara).

Set up a Turkish locale on Linux (glibc). I used Docker:

docker run -it ubuntu:24.04 bash
apt-get update && apt-get install -y locales
localedef -i tr_TR -f UTF-8 tr_TR.UTF-8

Build ccextractor there (linux/build), then run:

LC_ALL=C           ./ccextractor ankara.scc --sentencecap --capfile cities.txt -out=txt -o out.txt
LC_ALL=tr_TR.UTF-8 ./ccextractor ankara.scc --sentencecap --capfile cities.txt -out=txt -o out.txt
LC_ALL master this PR
C Ankara on Halloween. Ankara on Halloween.
en_US.UTF-8 Ankara on Halloween. Ankara on Halloween.
tr_TR.UTF-8 Ankara on halloween. Ankara on Halloween.

What's wrong

#2397 sorts the word lists in Rust, with ASCII case folding. The C code searches them with bsearch and strcasecmp. main() calls setlocale(LC_ALL, ""), so strcasecmp follows the user's locale. In tr_TR.UTF-8, glibc does not lowercase I. So "I", "I'd" and "Italy" sort before "April" in C, but after "Halloween" in the Rust list. With one extra word from --capfile, bsearch lands on one of those words and goes the wrong way. "Halloween" is then not found.

What I changed

  • main() calls ccx_encoders_helpers_perform_shellsort_words() right after ccxr_parse_parameters(). It sorts both lists with the same comparator that bsearch uses, in the active locale. The C parser did this before [FEAT] Removed C code already ported to Rust #1738.
  • Removed the Rust sort and its two tests. An ASCII sort can't match a locale-dependent strcasecmp.

How I tested

  • Ran the commands above on master and on this PR, in C, en_US.UTF-8 and tr_TR.UTF-8.
  • --profanity-file output is the same in all three locales.
  • cargo test --lib (434 passed), cargo fmt --check, cargo clippy -- -D warnings, clang-format.

Copilot AI balanced review requested due to automatic review settings November 12, 2026 04:17
Copilot AI reviewed Nov 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@ccextractor-bot

ccextractor-bot commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on linux. 172/238 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 26/28
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.

Compared with the tip of master — test 9984, commit 24d1e0a:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from: the same run as the tip of master (test 9984), so the comparison above already covers it.


No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

@ccextractor-bot

ccextractor-bot commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on windows. 172/238 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 26/28
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.

Compared with the tip of master — test 9985, commit 24d1e0a:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from: the same run as the tip of master (test 9985), so the comparison above already covers it.


No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Deepak-negi11 @ccextractor-bot