Skip to content

Commit

Permalink
chore: incorporate user resolutions in future chunk checks
Browse files Browse the repository at this point in the history
  • Loading branch information
drahnr committed Feb 12, 2023
1 parent 0e57540 commit ea60ff1
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 61 deletions.
11 changes: 6 additions & 5 deletions src/action/interactive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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 => {
Expand Down
68 changes: 29 additions & 39 deletions src/action/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Finish> {
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();
Expand Down
1 change: 1 addition & 0 deletions src/checker/dummy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl Checker for DummyChecker {
replacements,
chunk,
description: None,
checker_feedback_channel: None,
};
acc.push(suggestion);
}
Expand Down
67 changes: 52 additions & 15 deletions src/checker/hunspell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -107,22 +109,18 @@ pub fn consists_of_vulgar_fractions_or_emojis(word: &str) -> bool {
}

#[derive(Clone)]
struct HunspellSafe(Arc<Hunspell>);
struct HunspellSafe {
locked: Arc<Mutex<Hunspell>>,
}

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<Hunspell> for HunspellSafe {
fn from(hunspell: Hunspell) -> Self {
Self(Arc::new(hunspell))
Self {
locked: Arc::new(Mutex::new(hunspell)),
}
}
}

Expand Down Expand Up @@ -266,21 +264,53 @@ impl HunspellCheckerInner {
}

#[derive(Clone)]
pub struct HunspellChecker(pub Arc<HunspellCheckerInner>, pub Arc<Tokenizer>);
pub struct HunspellChecker {
pub inner: Arc<HunspellCheckerInner>,
pub tokenizer: Arc<Tokenizer>,
feedback_sender: Sender<String>,
feedback_receiver: Arc<Mutex<Receiver<String>>>,
}

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: &<HunspellChecker as Checker>::Config) -> Result<Self> {
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<String> {
self.feedback_sender.clone()
}
}

Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -368,6 +401,9 @@ impl Checker for HunspellChecker {
}
}
}
for item in acc.iter_mut() {
item.checker_feedback_channel.replace(self.sender());
}
Ok(acc)
}
}
Expand Down Expand Up @@ -419,6 +455,7 @@ fn obtain_suggestions<'s>(
replacements: replacements.clone(),
chunk,
description: Some("Possible spelling mistake found.".to_owned()),
checker_feedback_channel: None,
})
}
}
Expand Down
1 change: 1 addition & 0 deletions src/checker/nlprules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
);
}
Expand Down
1 change: 1 addition & 0 deletions src/reflow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ fn store_suggestion<'s>(
range,
replacements: vec![replacement],
span,
checker_feedback_channel: None,
};
suggestion
}),
Expand Down
54 changes: 52 additions & 2 deletions src/suggestion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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,
Expand All @@ -313,6 +313,51 @@ pub struct Suggestion<'s> {
pub replacements: Vec<String>,
/// Descriptive reason for the suggestion.
pub description: Option<String>,
/// Update the backend based on the decision
pub checker_feedback_channel: Option<std::sync::mpsc::Sender<String>>,
}

impl Hash for Suggestion<'_> {
fn hash<H: std::hash::Hasher>(&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<str>) {
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> {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -987,6 +1036,7 @@ mod tests {
range: 2..6,
replacements: vec!["whocares".to_owned()],
description: None,
checker_feedback_channel: None,
};

let suggestion = dbg!(suggestion);
Expand Down

0 comments on commit ea60ff1

Please sign in to comment.