From ea60ff125f6b4d4d57c013874108be019494915d Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Sun, 12 Feb 2023 16:11:29 +0100 Subject: [PATCH] chore: incorporate user resolutions in future chunk checks --- src/action/interactive.rs | 11 ++++--- src/action/mod.rs | 68 +++++++++++++++++---------------------- src/checker/dummy.rs | 1 + src/checker/hunspell.rs | 67 +++++++++++++++++++++++++++++--------- src/checker/nlprules.rs | 1 + src/reflow/mod.rs | 1 + src/suggestion.rs | 54 +++++++++++++++++++++++++++++-- 7 files changed, 142 insertions(+), 61 deletions(-) diff --git a/src/action/interactive.rs b/src/action/interactive.rs index c4c4be8c..6d5b02e3 100644 --- a/src/action/interactive.rs +++ b/src/action/interactive.rs @@ -167,6 +167,7 @@ where if self.is_ticked_entry() { BandAid::from((self.backticked_original.clone(), &self.suggestion.span)) } else if self.is_custom_entry() { + self.suggestion.feedback(&self.custom_replacement); BandAid::from((self.custom_replacement.clone(), &self.suggestion.span)) } else { let replacement = self @@ -500,16 +501,16 @@ impl UserPicked { // TODO make use of it let direction = Direction::Forward; 'outer: loop { - let opt_next = match direction { + let maybe_suggestion_next = match direction { Direction::Forward => suggestions_it.next(), // FIXME TODO this is just plain wrong Direction::Backward => suggestions_it.next_back(), }; - log::trace!("next() ---> {:?}", &opt_next); + log::trace!("next() ---> {:?}", &maybe_suggestion_next); - let (idx, suggestion) = match opt_next { - Some(x) => x, + let (idx, suggestion) = match maybe_suggestion_next { + Some(idx_suggestions) => idx_suggestions, None => match direction { Direction::Forward => { log::trace!("completed file, continue to next"); @@ -538,7 +539,7 @@ impl UserPicked { } UserSelection::SkipFile => break 'outer, UserSelection::Previous => { - log::warn!("Requires a iterator which works bidrectionally"); + log::warn!("Requires an iterator which works bi-directionally"); continue 'inner; } UserSelection::Help => { diff --git a/src/action/mod.rs b/src/action/mod.rs index 60675932..a823b037 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -349,51 +349,41 @@ impl Action { /// Run the requested action _interactively_, waiting for user input. async fn run_fix_interactive(self, documents: Documentation, config: Config) -> Result { - let n_cpus = num_cpus::get(); - - let checkers = Checkers::new(config)?; - let n = documents.entry_count(); log::debug!("Running checkers on all documents {}", n); - let mut pick_stream = stream::iter(documents.iter().enumerate()) - .map(|(mut idx, (origin, chunks))| { - // align the debug output with the user output - idx += 1; - log::trace!("Running checkers on {}/{},{:?}", idx, n, &origin); - let suggestions = checkers.check(origin, &chunks[..]); - async move { Ok::<_, color_eyre::eyre::Report>((idx, origin, suggestions?)) } - }) - .buffered(n_cpus) - .fuse(); + let checkers = Checkers::new(config.clone())?; let mut collected_picks = UserPicked::default(); - while let Some(result) = pick_stream.next().await { - match result { - Ok((idx, origin, suggestions)) => { - let (picked, user_sel) = - interactive::UserPicked::select_interactive(origin.clone(), suggestions)?; - - match user_sel { - UserSelection::Quit => break, - UserSelection::Abort => return Ok(Finish::Abort), - UserSelection::Nop if !picked.is_empty() => { - log::debug!( - "User picked patches to be applied for {}/{},{:?}", - idx, - n, - &origin - ); - collected_picks.extend(picked); - } - UserSelection::Nop => { - log::debug!("Nothing to do for {}/{},{:?}", idx, n, &origin); - } - _ => unreachable!( - "All other variants are only internal to `select_interactive`. qed" - ), + for (idx, (origin, chunks)) in documents.iter().enumerate() { + for chunk in chunks.into_iter().cloned() { + log::info!("Running checkers on {}/{},{:?}", idx, n, &origin); + + // Need to do it one by one, so the feedback is incorporated from the user picks, which is the next block + let chunk = [chunk]; + let suggestions = checkers.check(origin, &chunk[..])?; + + let (picked, user_sel) = + interactive::UserPicked::select_interactive(origin.clone(), suggestions)?; + + match user_sel { + UserSelection::Quit => break, + UserSelection::Abort => return Ok(Finish::Abort), + UserSelection::Nop if !picked.is_empty() => { + log::debug!( + "User picked patches to be applied for {}/{},{:?}", + idx, + n, + &origin + ); + collected_picks.extend(picked); + } + UserSelection::Nop => { + log::debug!("Nothing to do for {}/{},{:?}", idx, n, &origin); } + _ => unreachable!( + "All other variants are only internal to `select_interactive`. qed" + ), } - Err(e) => Err(e)?, } } let total = collected_picks.total_count(); diff --git a/src/checker/dummy.rs b/src/checker/dummy.rs index 36dc3185..cab1470a 100644 --- a/src/checker/dummy.rs +++ b/src/checker/dummy.rs @@ -60,6 +60,7 @@ impl Checker for DummyChecker { replacements, chunk, description: None, + checker_feedback_channel: None, }; acc.push(suggestion); } diff --git a/src/checker/hunspell.rs b/src/checker/hunspell.rs index e61c7aeb..88369337 100644 --- a/src/checker/hunspell.rs +++ b/src/checker/hunspell.rs @@ -19,7 +19,9 @@ use nlprule::Tokenizer; use std::io::{self, BufRead}; use std::path::{Path, PathBuf}; +use std::sync::mpsc::{self, Receiver, Sender}; use std::sync::Arc; +use std::sync::Mutex; use hunspell_rs::{CheckResult, Hunspell}; @@ -107,22 +109,18 @@ pub fn consists_of_vulgar_fractions_or_emojis(word: &str) -> bool { } #[derive(Clone)] -struct HunspellSafe(Arc); +struct HunspellSafe { + locked: Arc>, +} unsafe impl Send for HunspellSafe {} -// We only use it in RO so it's ok. unsafe impl Sync for HunspellSafe {} -impl std::ops::Deref for HunspellSafe { - type Target = Hunspell; - fn deref(&self) -> &Self::Target { - &self.0 - } -} - impl From for HunspellSafe { fn from(hunspell: Hunspell) -> Self { - Self(Arc::new(hunspell)) + Self { + locked: Arc::new(Mutex::new(hunspell)), + } } } @@ -266,21 +264,53 @@ impl HunspellCheckerInner { } #[derive(Clone)] -pub struct HunspellChecker(pub Arc, pub Arc); +pub struct HunspellChecker { + pub inner: Arc, + pub tokenizer: Arc, + feedback_sender: Sender, + feedback_receiver: Arc>>, +} impl std::ops::Deref for HunspellChecker { type Target = HunspellCheckerInner; fn deref(&self) -> &Self::Target { - self.0.deref() + self.inner.deref() } } impl HunspellChecker { + /// Create a new instance of the `Hunspell` backed spelling checker. pub fn new(config: &::Config) -> Result { let tokenizer = super::tokenizer::<&PathBuf>(None)?; let inner = HunspellCheckerInner::new(config)?; let hunspell = Arc::new(inner); - Ok(HunspellChecker(hunspell, tokenizer)) + let (feedback_sender, feedback_receiver) = mpsc::channel(); + let feedback_receiver = Arc::new(Mutex::new(feedback_receiver)); + + Ok(HunspellChecker { + inner: hunspell, + tokenizer, + feedback_sender, + feedback_receiver, + }) + } + + /// Continuosly update Tinhat with user feedback. + pub fn incorporate_custom_resolutions(&self) { + log::debug!("Check if custom user entry was selected, trying to acquire lock...."); + let feedback_receiver = self.feedback_receiver.lock().unwrap(); + log::debug!("Lock acquired"); + while let Some(word) = dbg!(feedback_receiver.try_recv()).ok().as_ref() { + let mut hunspell = self.inner.hunspell.locked.lock().unwrap(); + log::info!("Adding word >{word}< to hunspell (in memory only!)"); + hunspell.add(word); + assert_eq!(hunspell.check(word), CheckResult::FoundInDictionary); + } + } + + /// Moaria Tinhat + fn sender(&self) -> Sender { + self.feedback_sender.clone() } } @@ -305,9 +335,10 @@ impl Checker for HunspellChecker { let plain = chunk.erase_cmark(); log::trace!("{:?}", &plain); let txt = plain.as_str(); - let hunspell = &*self.hunspell.0; - 'tokenization: for range in apply_tokenizer(&self.1, txt) { + 'tokenization: for range in apply_tokenizer(&self.tokenizer, txt) { + self.incorporate_custom_resolutions(); + let word = sub_chars(txt, range.clone()); if range.len() == 1 && word @@ -318,6 +349,8 @@ impl Checker for HunspellChecker { { continue 'tokenization; } + + let hunspell = self.inner.hunspell.locked.lock().unwrap(); if self.transform_regex.is_empty() { obtain_suggestions( &plain, @@ -368,6 +401,9 @@ impl Checker for HunspellChecker { } } } + for item in acc.iter_mut() { + item.checker_feedback_channel.replace(self.sender()); + } Ok(acc) } } @@ -419,6 +455,7 @@ fn obtain_suggestions<'s>( replacements: replacements.clone(), chunk, description: Some("Possible spelling mistake found.".to_owned()), + checker_feedback_channel: None, }) } } diff --git a/src/checker/nlprules.rs b/src/checker/nlprules.rs index ae4215dd..24c375b9 100644 --- a/src/checker/nlprules.rs +++ b/src/checker/nlprules.rs @@ -147,6 +147,7 @@ fn check_chunk<'a>( replacements: replacements.iter().map(|x| x.clone()).collect(), chunk, description: Some(message.to_owned()), + checker_feedback_channel: None, }), ); } diff --git a/src/reflow/mod.rs b/src/reflow/mod.rs index d4fcb8f4..7a063928 100644 --- a/src/reflow/mod.rs +++ b/src/reflow/mod.rs @@ -365,6 +365,7 @@ fn store_suggestion<'s>( range, replacements: vec![replacement], span, + checker_feedback_channel: None, }; suggestion }), diff --git a/src/suggestion.rs b/src/suggestion.rs index 1fbd3221..465b39de 100644 --- a/src/suggestion.rs +++ b/src/suggestion.rs @@ -13,8 +13,8 @@ use crate::documentation::{CheckableChunk, ContentOrigin}; -use std::cmp; use std::convert::TryFrom; +use std::{cmp, hash::Hash}; use rayon::iter::{IntoParallelRefMutIterator, ParallelIterator}; @@ -296,7 +296,7 @@ pub fn condition_display_content( } /// A suggestion for certain offending span. -#[derive(Clone, Hash, PartialEq, Eq)] +#[derive(Clone)] pub struct Suggestion<'s> { /// Which checker suggested the change. pub detector: Detector, @@ -313,6 +313,51 @@ pub struct Suggestion<'s> { pub replacements: Vec, /// Descriptive reason for the suggestion. pub description: Option, + /// Update the backend based on the decision + pub checker_feedback_channel: Option>, +} + +impl Hash for Suggestion<'_> { + fn hash(&self, state: &mut H) { + self.detector.hash(state); + self.origin.hash(state); + self.chunk.hash(state); + self.span.hash(state); + self.range.hash(state); + self.replacements.hash(state); + self.description.hash(state); + } +} + +impl PartialEq for Suggestion<'_> { + fn eq(&self, other: &Self) -> bool { + self.detector == other.detector + && self.origin == other.origin + && self.chunk == other.chunk + && self.range == other.range + && self.span == other.span + && self.replacements == other.replacements + && self.description == other.description + } +} + +impl Eq for Suggestion<'_> {} + +impl<'s> Suggestion<'s> { + /// Return the user selection, or pass the custom one. + pub fn feedback(&self, s: impl AsRef) { + log::info!("Attempt to feedback custom user input"); + if let Some(ref feedback_sender) = self.checker_feedback_channel { + if let Err(e) = feedback_sender.send(s.as_ref().to_owned()) { + log::warn!( + "Failed to provide feedback to checker {} from {}: {:?}", + self.detector, + self.origin, + e + ); + } + } + } } impl<'s> fmt::Display for Suggestion<'s> { @@ -741,6 +786,7 @@ mod tests { "replacement_2".to_owned(), ], description: Some("Possible spelling mistake found.".to_owned()), + checker_feedback_channel: None, }; const EXPECTED: &str = r#"error: spellcheck(Dummy) @@ -788,6 +834,7 @@ mod tests { }, replacements: vec![], description: Some("Possible spelling mistake found.".to_owned()), + checker_feedback_channel: None, }; const EXPECTED: &str = r#"error: spellcheck(Dummy) @@ -864,6 +911,7 @@ mod tests { "replacement_2".to_owned(), ], description: Some("Possible spelling mistake found.".to_owned()), + checker_feedback_channel: None, }; const EXPECTED: &str = r#"error: spellcheck(Dummy) @@ -930,6 +978,7 @@ mod tests { "replacement_2".to_owned(), ], description: Some("Possible spelling mistake found.".to_owned()), + checker_feedback_channel: None, }; const EXPECTED: &str = r#"error: spellcheck(Dummy) @@ -987,6 +1036,7 @@ mod tests { range: 2..6, replacements: vec!["whocares".to_owned()], description: None, + checker_feedback_channel: None, }; let suggestion = dbg!(suggestion);