Skip to content

feat: acronym#35

Merged
wezm merged 2 commits intowezm:masterfrom
carlocorradini:acronym
Apr 8, 2025
Merged

feat: acronym#35
wezm merged 2 commits intowezm:masterfrom
carlocorradini:acronym

Conversation

@carlocorradini
Copy link
Copy Markdown
Contributor

@carlocorradini carlocorradini commented Mar 17, 2025

Fix #34

Copy link
Copy Markdown
Contributor

@alerque alerque left a comment

Choose a reason for hiding this comment

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

First, this is going to find false positives of any single words in parenthesis, e.g.:

this heading is a title (subject)

Secondly, anything along these lines would need to (in my opinion) gated behind an option of some kind.

@carlocorradini
Copy link
Copy Markdown
Contributor Author

@alerque Yes, that's why it's a draft. We can start with this and refine 🙌

@carlocorradini
Copy link
Copy Markdown
Contributor Author

carlocorradini commented Mar 17, 2025

First, this is going to find false positives of any single words in parenthesis, e.g.:

this heading is a title (subject)

Secondly, anything along these lines would need to (in my opinion) gated behind an option of some kind.

this heading is a title (subject)
is correctly transformed to
This Heading Is a Title (Subject)

To determine whether a word is an acronym, there are two checks:

  1. Check if the number of open and closed braces is equal
  2. Regex to check if a word between brackets is all uppercase and/or contains numbers: (?x)\A\(+[A-Z0-9]+\)+\z

@wezm
Copy link
Copy Markdown
Owner

wezm commented Mar 19, 2025

This seems reasonable from a quick glance.

@carlocorradini carlocorradini marked this pull request as ready for review March 19, 2025 07:32
@carlocorradini
Copy link
Copy Markdown
Contributor Author

@wezm @alerque Could you please share your opinions and suggestions? Thanks 🥳☺️

@wezm
Copy link
Copy Markdown
Owner

wezm commented Mar 20, 2025

Yep I plan to review properly this weekend.

Copy link
Copy Markdown
Owner

@wezm wezm left a comment

Choose a reason for hiding this comment

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

This looks good to me. Did you have any outstanding concerns @alerque?

@alerque
Copy link
Copy Markdown
Contributor

alerque commented Mar 22, 2025

Yes. It doesn't seem to be optional yet, and it does still look like it will be easy to trip with false positives.

WHAT ABOUT TITLE CASING THIS (SAMPLE) INPUT?

@wezm
Copy link
Copy Markdown
Owner

wezm commented Mar 23, 2025

That seems to produce the same output with the current version and this branch?

   Compiling titlecase v3.4.0 (/home/wmoore/Projects/titlecase)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.23s
 10:07  ~  Projects  titlecase  titlecase <<< 'WHAT ABOUT TITLE CASING THIS (SAMPLE) INPUT?'
What About Title Casing This (Sample) Input?
 10:07  ~  Projects  titlecase  target/debug/titlecase <<< 'WHAT ABOUT TITLE CASING THIS (SAMPLE) INPUT?'
What About Title Casing This (Sample) Input?

@carlocorradini
Copy link
Copy Markdown
Contributor Author

@wezm This is because of the following line:

titlecase/src/lib.rs

Lines 113 to 117 in e87fa34

let trimmed_input = if trimmed_input.chars().any(|ch| ch.is_lowercase()) {
Cow::from(trimmed_input)
} else {
Cow::from(trimmed_input.to_lowercase())
};

@wezm
Copy link
Copy Markdown
Owner

wezm commented Mar 24, 2025

@carlocorradini yes, it seems to working as desired.

@wezm wezm merged commit 97faf73 into wezm:master Apr 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow acronym/abbreviation

3 participants