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

chore: incorporate user resolutions in future chunk checks #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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