Skip to content

Commit

Permalink
fzf bugfix: find by hash instead of alias
Browse files Browse the repository at this point in the history
  • Loading branch information
EricDriussi committed Apr 24, 2024
1 parent f8b390d commit d2385c7
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 13 deletions.
12 changes: 11 additions & 1 deletion src/authors/author.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::common::conf;
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};

#[derive(Debug, PartialEq)]
pub struct Author {
Expand Down Expand Up @@ -27,9 +29,17 @@ impl Author {
pub fn name(&self) -> String {
self.name.clone()
}

pub fn hash(&self) -> u64 {
let mut hasher = DefaultHasher::new();
let to_hash = format!("{}{}", self.alias, self.name);
to_hash.hash(&mut hasher);
hasher.finish()
}
}

pub trait AuthorsProvider {
fn find(&self, aliases: &[String]) -> Vec<Author>;
fn find_by_aliases(&self, aliases: &[String]) -> Vec<Author>;
fn find_by_hashes(&self, hashes: &[u64]) -> Vec<Author>;
fn all(&self) -> Vec<Author>;
}
17 changes: 17 additions & 0 deletions src/authors/author_should.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::{authors::author::Author, common::conf};
use parameterized::parameterized;
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};

#[test]
fn present_a_co_author_compliant_signature() {
Expand Down Expand Up @@ -27,6 +29,15 @@ fn get_name() {
assert_eq!(author.name(), name);
}

#[test]
fn get_hash() {
let alias = "a";
let name = "alice";
let author = Author::from(alias, name, "[email protected]");

assert_eq!(author.hash(), hash_of(&format!("{alias}{name}")));
}

#[test]
fn be_equal_to_another_author_with_same_data() {
let alias = "a";
Expand All @@ -47,3 +58,9 @@ fn be_equal_to_another_author_with_same_data() {
fn not_be_equal_to_another_author_with_different_data(different_author: Author) {
assert_ne!(Author::from("a", "alice", "[email protected]"), different_author);
}

fn hash_of(str: &str) -> u64 {
let mut hasher = DefaultHasher::new();
str.hash(&mut hasher);
hasher.finish()
}
10 changes: 9 additions & 1 deletion src/authors/csv/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,22 @@ impl CSVProvider {
}

impl AuthorsProvider for CSVProvider {
fn find(&self, aliases: &[String]) -> Vec<Author> {
fn find_by_aliases(&self, aliases: &[String]) -> Vec<Author> {
self.lines
.iter()
.filter_map(|line| mapper::to_author(line.as_str()))
.filter(|author| aliases.contains(&author.alias()))
.collect()
}

fn find_by_hashes(&self, hashes: &[u64]) -> Vec<Author> {
self.lines
.iter()
.filter_map(|line| mapper::to_author(line.as_str()))
.filter(|author| hashes.contains(&author.hash()))
.collect()
}

fn all(&self) -> Vec<Author> {
self.lines
.iter()
Expand Down
36 changes: 33 additions & 3 deletions src/authors/csv/provider_should_give.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use super::provider::LoadMode;
use crate::authors::author::AuthorsProvider;
use crate::authors::csv::provider::CSVProvider;
use crate::common::fs::file_reader::MockReader;
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};

#[test]
fn all_authors_in_file() {
Expand All @@ -22,7 +24,7 @@ fn only_author_matching_an_alias() {
"b,username,[email protected]".to_string(),
]);

let retrieved_authors = provider.find(&["a".to_string()]);
let retrieved_authors = provider.find_by_aliases(&["a".to_string()]);

assert_eq!(retrieved_authors.len(), 1);
}
Expand All @@ -35,7 +37,7 @@ fn all_authors_matching_an_alias() {
"b,username2,[email protected]".to_string(),
]);

let retrieved_authors = provider.find(&["b".to_string()]);
let retrieved_authors = provider.find_by_aliases(&["b".to_string()]);

assert_eq!(retrieved_authors.len(), 2);
}
Expand All @@ -44,7 +46,29 @@ fn all_authors_matching_an_alias() {
fn no_author_when_alias_doesnt_match() {
let provider = csv_provider_with(vec!["a,Name Surname,[email protected]".to_string()]);

let retrieved_authors = provider.find(&["z".to_string()]);
let retrieved_authors = provider.find_by_aliases(&["z".to_string()]);

assert_eq!(retrieved_authors.len(), 0);
}

#[test]
fn only_author_matching_a_hash() {
let provider = csv_provider_with(vec![
"a,Name Surname,[email protected]".to_string(),
"a,Another Name Surname,[email protected]".to_string(),
"b,username,[email protected]".to_string(),
]);

let retrieved_authors = provider.find_by_hashes(&[hash_of("aName Surname")]);

assert_eq!(retrieved_authors.len(), 1);
}

#[test]
fn no_author_when_hash_doesnt_match() {
let provider = csv_provider_with(vec!["a,Name Surname,[email protected]".to_string()]);

let retrieved_authors = provider.find_by_hashes(&[hash_of("zIrrelevant")]);

assert_eq!(retrieved_authors.len(), 0);
}
Expand All @@ -60,3 +84,9 @@ fn csv_provider_with(authors: Vec<String>) -> CSVProvider {
})
.expect("Could not setup AuthorsProvider for test")
}

fn hash_of(str: &str) -> u64 {
let mut hasher = DefaultHasher::new();
str.hash(&mut hasher);
hasher.finish()
}
9 changes: 7 additions & 2 deletions src/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl Orchestrator {
if self.args.fzf {
let found_authors: Vec<_> = self
.provider
.find(&self.cli.fzf_prompt(&all_authors)?)
.find_by_hashes(&self.cli.fzf_prompt(&all_authors)?)
.iter()
.map(Author::signature)
.collect();
Expand All @@ -54,7 +54,12 @@ impl Orchestrator {
Some(list) => list.split(',').map(ToString::to_string).collect::<Vec<String>>(),
None => self.cli.aliases_prompt(&all_authors)?,
};
let found_authors: Vec<_> = self.provider.find(&aliases).iter().map(Author::signature).collect();
let found_authors: Vec<_> = self
.provider
.find_by_aliases(&aliases)
.iter()
.map(Author::signature)
.collect();

if self.args.sort {
Ok(Self::sort(found_authors))
Expand Down
18 changes: 12 additions & 6 deletions src/ui/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@ use crate::authors::author::Author;
use crate::common::runner::Runner;
use crate::Result;
use colored::Colorize;
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};
use std::io::Write;

const FZF_SEPARATOR: &str = " - ";

pub struct Cli {
reader: Box<dyn InputReader>,
runner: Box<dyn Runner>,
Expand Down Expand Up @@ -37,7 +41,7 @@ impl Cli {
Ok(input.trim().to_string())
}

pub fn fzf_prompt(&self, authors: &[Author]) -> Result<Vec<String>> {
pub fn fzf_prompt(&self, authors: &[Author]) -> Result<Vec<u64>> {
let mut fzf_proc = self
.runner
.attach("fzf", &["--multi".to_string(), "--ansi".to_string()])?;
Expand All @@ -53,16 +57,18 @@ impl Cli {
let output = fzf_proc
.wait_with_output()
.map_err(|_| UiError::Fzf("Could not read output".to_string()))?;
let selected_aliases: Vec<String> = String::from_utf8_lossy(&output.stdout)
let selected_aliases: Vec<u64> = String::from_utf8_lossy(&output.stdout)
.lines()
.map(Self::extract_alias)
.map(|line| Self::hash_of(&line.replace(FZF_SEPARATOR, "")))
.collect();

Ok(selected_aliases)
}

fn extract_alias(line: &str) -> String {
line.split_whitespace().next().unwrap_or_default().to_string()
fn hash_of(str: &str) -> u64 {
let mut hasher = DefaultHasher::new();
str.hash(&mut hasher);
hasher.finish()
}

fn prettify_authors(authors: &[Author]) -> String {
Expand All @@ -80,6 +86,6 @@ impl Cli {
}

fn fzf_format(author: &Author) -> String {
format!("{} - {}", author.alias().blue(), author.name())
format!("{}{}{}", author.alias().blue(), FZF_SEPARATOR, author.name())
}
}

0 comments on commit d2385c7

Please sign in to comment.