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

Load bad words when loading good words in SQ #1254

Merged
merged 2 commits into from
Sep 24, 2023

Conversation

windymilla
Copy link
Collaborator

When adding good_words.txt to the project dictionary in Spell Query, also load bad_words.txt if it exists.
Always report bad words as bad spellings, even if they are in the dictionary or good words file.
Mark bad words with asterisks in SQ report and report count of total bad word occurrences in file at top of dialog (unless zero)

When adding `good_words.txt` to the project dictionary in Spell
Query, also load `bad_words.txt` if it exists.
Always report bad words as bad spellings, even if they are in the
dictionary or good words file.
Mark bad words with asterisks in SQ report
Since bad words are...bad, display how many there are, so PPer is
incentivized to locate and correct them.

Renaming of old `badwordfreq` variable to `badspellingfreq` to
avoid confusion with count of bad words from the BWL
@windymilla windymilla requested review from cpeel and srjfoo September 19, 2023 20:44
@windymilla
Copy link
Collaborator Author

Testing notes: create files good_words.txt and bad_words.txt with single word per line. Use Spell Query to spell check a file, then load the good/bad words and re-check. Bad words should always be reported, even if you try to add them to the project dictionary.

@windymilla windymilla linked an issue Sep 19, 2023 that may be closed by this pull request
Copy link
Member

@cpeel cpeel left a comment

Choose a reason for hiding this comment

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

Noted one minor thing to think about, but LGTM!

my %sqbadspellingfreq = ();
my %sqbadwordcount = ();

# Codes returned by spellquerywordok()
Copy link
Member

Choose a reason for hiding this comment

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

Love the use of these named "constants" -- very easy to understand what's going on 👍

sub spellmyaddword {
my $textwindow = $::textwindow;
my $term = shift;
my $bad = shift;
Copy link
Member

Choose a reason for hiding this comment

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

This arg type is a bit confusing to me. I had first thought it was a true/false value (since that's how it's evaluated here) but we're passing in the string bad which is evaluating to true. It all works but it feels unintuitive.

I'm not exactly sure if it's better to:

  • keep it as-is
  • change it to $is_bad and pass in a 0 / 1
  • change it to $word_type that if bad is added to the bad list

Not wedded to any solution (or changing it at all for that matter) but thought it worth mentioning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I hadn't thought about it being confusing before. I'm afraid I've littered the code with similar optional arguments: normally omitted when the sub is called, but in some places a string is passed in, which as you say evaluates to true. Up until now, I've managed to sneak them past your eagle eye , but I've obviously ridden my luck once too often 🤣

My thinking, for what it's worth, is that it's easier to understand what's going on when it's called: spellmyaddword($word, 'bad') compared with spellmyaddword($word, 1), but I accept it would be a little safer to then check within the routine that the input argument was actually 'bad'. As it stands, if future windy copy/pastes a function call to add a bad word, but wants to add a good word, they might think that changing 'bad' to 'good' would do that, and would be disappointed with past windy's thoughtless use of Perl's automatic type conversion.

That said, since the same thing is in lots of places 😊, I think I'll keep it as-is, but consider a global tidy up at some point to fix "optional string argument treated as boolean".

Copy link
Member

Choose a reason for hiding this comment

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

We're on the same page and agreed we shouldn't deviate from an existing code convention already in use.

@windymilla windymilla merged commit 7aadeac into DistributedProofreaders:master Sep 24, 2023
1 check passed
@windymilla windymilla deleted the sq-bad-words branch September 24, 2023 14:55
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 user to load bad_words.txt file in Spell Query
3 participants