From ed01f8b463322b89fbe9abcfa528f9be74a077b8 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 5 Feb 2019 19:58:08 +0100 Subject: [PATCH 1/5] Make annotations unnamed (automatically UUID named) It makes no sense to name annotations, a user only cares about whether there are annotations or not, or their contents. A name has no meaning in this context. Signed-off-by: Matthias Beyer --- lib/entry/libimagentryannotation/Cargo.toml | 3 ++ .../src/annotateable.rs | 37 +++++++++---------- .../src/annotation_fetcher.rs | 16 +++----- lib/entry/libimagentryannotation/src/lib.rs | 4 +- 4 files changed, 29 insertions(+), 31 deletions(-) diff --git a/lib/entry/libimagentryannotation/Cargo.toml b/lib/entry/libimagentryannotation/Cargo.toml index a677a6b3..166eef92 100644 --- a/lib/entry/libimagentryannotation/Cargo.toml +++ b/lib/entry/libimagentryannotation/Cargo.toml @@ -25,6 +25,9 @@ toml = "0.4" toml-query = "0.8" failure = "0.1" failure_derive = "0.1" +uuid = { version = "0.7", features = ["v4"] } +log = "0.4.0" + libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagerror = { version = "0.10.0", path = "../../../lib/core/libimagerror" } diff --git a/lib/entry/libimagentryannotation/src/annotateable.rs b/lib/entry/libimagentryannotation/src/annotateable.rs index 45da1dca..fdc12821 100644 --- a/lib/entry/libimagentryannotation/src/annotateable.rs +++ b/lib/entry/libimagentryannotation/src/annotateable.rs @@ -18,6 +18,7 @@ // use toml::Value; +use uuid::Uuid; use libimagstore::store::Entry; use libimagstore::store::FileLockEntry; @@ -28,7 +29,6 @@ use libimagentrylink::internal::InternalLinker; use libimagentryutil::isa::Is; use libimagentryutil::isa::IsKindHeaderPathProvider; -use toml_query::read::TomlValueReadTypeExt; use toml_query::insert::TomlValueInsertExt; use failure::Fallible as Result; @@ -37,9 +37,10 @@ use failure::Error; use failure::err_msg; use iter::*; +use module_path::ModuleEntryPath; pub trait Annotateable { - fn annotate<'a>(&mut self, store: &'a Store, ann_name: &str) -> Result>; + fn annotate<'a>(&mut self, store: &'a Store) -> Result>; fn denotate<'a>(&mut self, store: &'a Store, ann_name: &str) -> Result>>; fn annotations<'a>(&self, store: &'a Store) -> Result>; fn is_annotation(&self) -> Result; @@ -50,9 +51,11 @@ provide_kindflag_path!(IsAnnotation, "annotation.is_annotation"); impl Annotateable for Entry { /// Annotate an entry, returns the new entry which is used to annotate - fn annotate<'a>(&mut self, store: &'a Store, ann_name: &str) -> Result> { - use module_path::ModuleEntryPath; - store.retrieve(ModuleEntryPath::new(ann_name).into_storeid()?) + fn annotate<'a>(&mut self, store: &'a Store) -> Result> { + let ann_name = Uuid::new_v4().to_hyphenated().to_string(); + debug!("Creating annotation with name = {}", ann_name); + + store.retrieve(ModuleEntryPath::new(ann_name.clone()).into_storeid()?) .and_then(|mut anno| { { let _ = anno.set_isflag::()?; @@ -70,23 +73,17 @@ impl Annotateable for Entry { }) } - /// Checks the current entry for all annotations and removes the one where the name is - /// `ann_name`, which is then returned + // Removes the annotation `ann_name` from the current entry. + // Fails if there's no such annotation entry or if the link to that annotation entry does not + // exist. fn denotate<'a>(&mut self, store: &'a Store, ann_name: &str) -> Result>> { - for annotation in self.annotations(store)? { - let mut anno = annotation?; - let name = match anno.get_header().read_string("annotation.name")? { - Some(ref name) => name.clone(), - None => continue, - }; - - if name == ann_name { - let _ = self.remove_internal_link(&mut anno)?; - return Ok(Some(anno)); - } + if let Some(mut annotation) = store.get(ModuleEntryPath::new(ann_name).into_storeid()?)? { + let _ = self.remove_internal_link(&mut annotation)?; + Ok(Some(annotation)) + } else { + // error: annotation does not exist + Err(format_err!("Annotation '{}' does not exist", ann_name)).map_err(Error::from) } - - Ok(None) } /// Get all annotations of an entry diff --git a/lib/entry/libimagentryannotation/src/annotation_fetcher.rs b/lib/entry/libimagentryannotation/src/annotation_fetcher.rs index ac903807..dd6c2222 100644 --- a/lib/entry/libimagentryannotation/src/annotation_fetcher.rs +++ b/lib/entry/libimagentryannotation/src/annotation_fetcher.rs @@ -18,21 +18,17 @@ // use libimagstore::store::Store; +use libimagstore::storeid::StoreIdIterator; use failure::Fallible as Result; -use iter::*; - -pub trait AnnotationFetcher<'a> { - - fn all_annotations(&'a self) -> Result>; +pub trait AnnotationFetcher { + fn all_annotations(&self) -> Result; } -impl<'a> AnnotationFetcher<'a> for Store { - - fn all_annotations(&'a self) -> Result> { - Ok(AnnotationIter::new(self.entries()?.without_store(), self)) +impl<'a> AnnotationFetcher for Store { + fn all_annotations(&self) -> Result { + self.entries().map(|iter| iter.in_collection("annotation").without_store()) } - } diff --git a/lib/entry/libimagentryannotation/src/lib.rs b/lib/entry/libimagentryannotation/src/lib.rs index 24de090b..e3be82b9 100644 --- a/lib/entry/libimagentryannotation/src/lib.rs +++ b/lib/entry/libimagentryannotation/src/lib.rs @@ -39,7 +39,9 @@ extern crate toml; extern crate toml_query; -extern crate failure; +#[macro_use] extern crate failure; +#[macro_use] extern crate log; +extern crate uuid; #[macro_use] extern crate libimagstore; extern crate libimagerror; From ab5078f1119156ceddf7cac37960aa09ebe507d3 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 5 Feb 2019 20:08:08 +0100 Subject: [PATCH 2/5] Rewrite "add annotation" command Because before we created a new annotation for each ID to be annotated, which is not the expected behaviour. Now we create one annotation object and then link it to all IDs which are provided on the commandline. Also, the annotation name is printed. Signed-off-by: Matthias Beyer --- bin/core/imag-annotate/Cargo.toml | 1 + bin/core/imag-annotate/src/main.rs | 59 ++++++++++++++++++++++-------- bin/core/imag-annotate/src/ui.rs | 7 ---- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/bin/core/imag-annotate/Cargo.toml b/bin/core/imag-annotate/Cargo.toml index 072beeec..67e72a31 100644 --- a/bin/core/imag-annotate/Cargo.toml +++ b/bin/core/imag-annotate/Cargo.toml @@ -32,6 +32,7 @@ libimagstore = { version = "0.10.0", path = "../../../lib/core/libimag libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } libimagerror = { version = "0.10.0", path = "../../../lib/core/libimagerror" } libimagentryannotation = { version = "0.10.0", path = "../../../lib/entry/libimagentryannotation" } +libimagentrylink = { version = "0.10.0", path = "../../../lib/entry/libimagentrylink" } libimagentryedit = { version = "0.10.0", path = "../../../lib/entry/libimagentryedit" } libimagutil = { version = "0.10.0", path = "../../../lib/etc/libimagutil" } diff --git a/bin/core/imag-annotate/src/main.rs b/bin/core/imag-annotate/src/main.rs index da965d7c..8699463a 100644 --- a/bin/core/imag-annotate/src/main.rs +++ b/bin/core/imag-annotate/src/main.rs @@ -37,7 +37,9 @@ extern crate clap; #[macro_use] extern crate log; +#[macro_use] extern crate failure; +extern crate toml_query; extern crate libimagentryannotation; extern crate libimagentryedit; @@ -45,6 +47,7 @@ extern crate libimagerror; #[macro_use] extern crate libimagrt; extern crate libimagstore; extern crate libimagutil; +extern crate libimagentrylink; use std::io::Write; @@ -57,9 +60,12 @@ use libimagerror::trace::MapErrTrace; use libimagerror::exit::ExitUnwrap; use libimagerror::io::ToExitCode; use libimagerror::errors::ErrorMsg as EM; +use libimagerror::iter::TraceIterator; use libimagrt::runtime::Runtime; use libimagrt::setup::generate_runtime_setup; use libimagstore::store::FileLockEntry; +use libimagstore::iter::get::StoreIdGetIteratorExtension; +use libimagentrylink::internal::InternalLinker; mod ui; @@ -89,23 +95,45 @@ fn main() { } fn add(rt: &Runtime) { - let scmd = rt.cli().subcommand_matches("add").unwrap(); // safed by main() - let annotation_name = scmd.value_of("annotation_name").unwrap(); // safed by clap - let ids = rt.ids::<::ui::PathProvider>().map_err_trace_exit_unwrap(1); + let scmd = rt.cli().subcommand_matches("add").unwrap(); // safed by main() + let mut ids = rt.ids::<::ui::PathProvider>().map_err_trace_exit_unwrap(1).into_iter(); - ids.into_iter().for_each(|id| { - let _ = rt.store() - .get(id.clone()) + if let Some(first) = ids.next() { + let mut annotation = rt.store() + .get(first.clone()) .map_err_trace_exit_unwrap(1) - .ok_or_else(|| EM::EntryNotFound(id.local_display_string())) + .ok_or_else(|| EM::EntryNotFound(first.local_display_string())) .map_err(Error::from) .map_err_trace_exit_unwrap(1) - .annotate(rt.store(), annotation_name) - .map_err_trace_exit_unwrap(1) - .edit_content(&rt) + .annotate(rt.store()) .map_err_trace_exit_unwrap(1); - }) + let _ = annotation.edit_content(&rt).map_err_trace_exit_unwrap(1); + + for id in ids { + let mut entry = rt.store().get(id.clone()) + .map_err_trace_exit_unwrap(1) + .ok_or_else(|| format_err!("Not found: {}", id.local_display_string())) + .map_err_trace_exit_unwrap(1); + + let _ = entry.add_internal_link(&mut annotation).map_err_trace_exit_unwrap(1); + } + + if let Some(annotation_id) = annotation + .get_header() + .read_string("annotation.name") + .map_err_trace_exit_unwrap(1) + { + let _ = writeln!(rt.stdout(), "Name of the annotation: {}", annotation_id) + .to_exit_code() + .unwrap_or_exit(1); + } else { + error!("Unnamed annotation: {:?}", annotation.get_location()); + error!("This is most likely a BUG, please report!"); + } + } else { + debug!("No entries to annotate"); + } } fn remove(rt: &Runtime) { @@ -164,11 +192,12 @@ fn list(rt: &Runtime) { .map_err_trace_exit_unwrap(1) .annotations(rt.store()) .map_err_trace_exit_unwrap(1) + .into_get_iter(rt.store()) + .trace_unwrap_exit(1) + .map(|opt| opt.ok_or_else(|| format_err!("Cannot find entry"))) + .trace_unwrap_exit(1) .enumerate() - .map(|(i, a)| { - list_annotation(&rt, i, a.map_err_trace_exit_unwrap(1), with_text) - }) - .collect::>(); + .for_each(|(i, entry)| list_annotation(&rt, i, entry, with_text)); }); } else { // ids.len() == 0 // show them all diff --git a/bin/core/imag-annotate/src/ui.rs b/bin/core/imag-annotate/src/ui.rs index 2a3b722f..ab17f744 100644 --- a/bin/core/imag-annotate/src/ui.rs +++ b/bin/core/imag-annotate/src/ui.rs @@ -38,13 +38,6 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { .multiple(false) .help("The entry to add the latitude/longitude to") .value_name("ENTRY")) - .arg(Arg::with_name("annotation_name") - .index(2) - .takes_value(true) - .required(true) - .multiple(false) - .help("Name of the new annotation") - .value_name("NAME")) ) .subcommand(SubCommand::with_name("remove") From c84258da3d2a5f62cc6cfae3c0c04a933c1fbb3d Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 5 Feb 2019 20:12:19 +0100 Subject: [PATCH 3/5] Add flag to not print name of annotation Signed-off-by: Matthias Beyer --- bin/core/imag-annotate/src/main.rs | 26 +++++++++++++++----------- bin/core/imag-annotate/src/ui.rs | 9 +++++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/bin/core/imag-annotate/src/main.rs b/bin/core/imag-annotate/src/main.rs index 8699463a..03153fb8 100644 --- a/bin/core/imag-annotate/src/main.rs +++ b/bin/core/imag-annotate/src/main.rs @@ -52,6 +52,7 @@ extern crate libimagentrylink; use std::io::Write; use failure::Error; +use toml_query::read::TomlValueReadTypeExt; use libimagentryannotation::annotateable::*; use libimagentryannotation::annotation_fetcher::*; @@ -119,17 +120,20 @@ fn add(rt: &Runtime) { let _ = entry.add_internal_link(&mut annotation).map_err_trace_exit_unwrap(1); } - if let Some(annotation_id) = annotation - .get_header() - .read_string("annotation.name") - .map_err_trace_exit_unwrap(1) - { - let _ = writeln!(rt.stdout(), "Name of the annotation: {}", annotation_id) - .to_exit_code() - .unwrap_or_exit(1); - } else { - error!("Unnamed annotation: {:?}", annotation.get_location()); - error!("This is most likely a BUG, please report!"); + if !scmd.is_present("dont-print-name") { + if let Some(annotation_id) = annotation + .get_header() + .read_string("annotation.name") + .map_err(Error::from) + .map_err_trace_exit_unwrap(1) + { + let _ = writeln!(rt.stdout(), "Name of the annotation: {}", annotation_id) + .to_exit_code() + .unwrap_or_exit(); + } else { + error!("Unnamed annotation: {:?}", annotation.get_location()); + error!("This is most likely a BUG, please report!"); + } } } else { debug!("No entries to annotate"); diff --git a/bin/core/imag-annotate/src/ui.rs b/bin/core/imag-annotate/src/ui.rs index ab17f744..024d6a3d 100644 --- a/bin/core/imag-annotate/src/ui.rs +++ b/bin/core/imag-annotate/src/ui.rs @@ -31,6 +31,15 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { .subcommand(SubCommand::with_name("add") .about("Add annotation to an entry") .version("0.1") + + .arg(Arg::with_name("dont-print-name") + .short("N") + .long("no-name") + .takes_value(false) + .required(false) + .multiple(false) + .help("Do not print the name of the annotation after annotating.") + ) .arg(Arg::with_name("entry") .index(1) .takes_value(true) From d8cd10a384e3cb8674214b68a83c690d927b48ee Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 10 Feb 2019 00:38:52 +0100 Subject: [PATCH 4/5] Remove iterator types With this patch, libimagentryannotation does not have special iterator types anymore. This makes the whole thing more comfortable to use. In imag-annotate, the parameter for the functioncall was removed. Signed-off-by: Matthias Beyer --- bin/core/imag-annotate/src/main.rs | 2 +- .../src/annotateable.rs | 13 ++-- lib/entry/libimagentryannotation/src/iter.rs | 77 ------------------- lib/entry/libimagentryannotation/src/lib.rs | 1 - 4 files changed, 9 insertions(+), 84 deletions(-) delete mode 100644 lib/entry/libimagentryannotation/src/iter.rs diff --git a/bin/core/imag-annotate/src/main.rs b/bin/core/imag-annotate/src/main.rs index 03153fb8..ae4dea6b 100644 --- a/bin/core/imag-annotate/src/main.rs +++ b/bin/core/imag-annotate/src/main.rs @@ -194,7 +194,7 @@ fn list(rt: &Runtime) { .ok_or_else(|| EM::EntryNotFound(id.local_display_string())) .map_err(Error::from) .map_err_trace_exit_unwrap(1) - .annotations(rt.store()) + .annotations() .map_err_trace_exit_unwrap(1) .into_get_iter(rt.store()) .trace_unwrap_exit(1) diff --git a/lib/entry/libimagentryannotation/src/annotateable.rs b/lib/entry/libimagentryannotation/src/annotateable.rs index fdc12821..3c915846 100644 --- a/lib/entry/libimagentryannotation/src/annotateable.rs +++ b/lib/entry/libimagentryannotation/src/annotateable.rs @@ -36,13 +36,12 @@ use failure::ResultExt; use failure::Error; use failure::err_msg; -use iter::*; use module_path::ModuleEntryPath; pub trait Annotateable { fn annotate<'a>(&mut self, store: &'a Store) -> Result>; fn denotate<'a>(&mut self, store: &'a Store, ann_name: &str) -> Result>>; - fn annotations<'a>(&self, store: &'a Store) -> Result>; + fn annotations(&self) -> Result; fn is_annotation(&self) -> Result; } @@ -87,10 +86,14 @@ impl Annotateable for Entry { } /// Get all annotations of an entry - fn annotations<'a>(&self, store: &'a Store) -> Result> { + fn annotations(&self) -> Result { self.get_internal_links() - .map(|iter| StoreIdIterator::new(Box::new(iter.map(|e| e.get_store_id().clone()).map(Ok)))) - .map(|i| AnnotationIter::new(i, store)) + .map(|it| { + it.filter(|link| link.get_store_id().is_in_collection(&["annotation"])) + .map(|link| Ok(link.get_store_id().clone())) + }) + .map(Box::new) + .map(|inner| StoreIdIterator::new(inner)) } fn is_annotation(&self) -> Result { diff --git a/lib/entry/libimagentryannotation/src/iter.rs b/lib/entry/libimagentryannotation/src/iter.rs deleted file mode 100644 index e7601a9c..00000000 --- a/lib/entry/libimagentryannotation/src/iter.rs +++ /dev/null @@ -1,77 +0,0 @@ -// -// imag - the personal information management suite for the commandline -// Copyright (C) 2015-2019 Matthias Beyer and contributors -// -// This library is free software; you can redistribute it and/or -// modify it under the terms of the GNU Lesser General Public -// License as published by the Free Software Foundation; version -// 2.1 of the License. -// -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -// Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public -// License along with this library; if not, write to the Free Software -// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -// - -use toml_query::read::TomlValueReadTypeExt; - -use libimagstore::store::Store; -use libimagstore::store::FileLockEntry; -use libimagstore::storeid::StoreIdIterator; -use libimagerror::errors::ErrorMsg as EM; - -use failure::Fallible as Result; -use failure::ResultExt; -use failure::Error; -use failure::err_msg; - -#[derive(Debug)] -pub struct AnnotationIter<'a>(StoreIdIterator, &'a Store); - -impl<'a> AnnotationIter<'a> { - - pub fn new(iter: StoreIdIterator, store: &'a Store) -> AnnotationIter<'a> { - AnnotationIter(iter, store) - } - -} - -impl<'a> Iterator for AnnotationIter<'a> { - type Item = Result>; - - fn next(&mut self) -> Option { - loop { - match self.0.next() { - None => return None, // iterator consumed - Some(Err(e)) => return Some(Err(e).map_err(Error::from)), - Some(Ok(id)) => match self.1.get(id) { - Err(e) => { - return Some(Err(e) - .context(err_msg("Store read error")) - .map_err(Error::from)) - }, - Ok(Some(entry)) => { - match entry - .get_header() - .read_bool("annotation.is_annotation") - .context(EM::EntryHeaderReadError) - .map_err(Error::from) - { - Ok(None) => continue, // not an annotation - Ok(Some(false)) => continue, - Ok(Some(true)) => return Some(Ok(entry)), - Err(e) => return Some(Err(e)), - } - }, - Ok(None) => continue, - } - } - } - } - -} - diff --git a/lib/entry/libimagentryannotation/src/lib.rs b/lib/entry/libimagentryannotation/src/lib.rs index e3be82b9..92da306f 100644 --- a/lib/entry/libimagentryannotation/src/lib.rs +++ b/lib/entry/libimagentryannotation/src/lib.rs @@ -52,5 +52,4 @@ module_entry_path_mod!("annotations"); pub mod annotateable; pub mod annotation_fetcher; -pub mod iter; From 53022443abf4970bea452a87aa90cb66893f1e63 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 10 Feb 2019 00:21:01 +0100 Subject: [PATCH 5/5] Rewrite iteration This patch rewrites how imag-annotate iterates over the ids to process to be more easily to understand. Signed-off-by: Matthias Beyer --- bin/core/imag-annotate/src/main.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/core/imag-annotate/src/main.rs b/bin/core/imag-annotate/src/main.rs index ae4dea6b..afb5ce6e 100644 --- a/bin/core/imag-annotate/src/main.rs +++ b/bin/core/imag-annotate/src/main.rs @@ -205,15 +205,15 @@ fn list(rt: &Runtime) { }); } else { // ids.len() == 0 // show them all - let _ = rt - .store() + rt.store() .all_annotations() .map_err_trace_exit_unwrap(1) + .into_get_iter(rt.store()) + .trace_unwrap_exit(1) + .map(|opt| opt.ok_or_else(|| format_err!("Cannot find entry"))) + .trace_unwrap_exit(1) .enumerate() - .map(|(i, a)| { - list_annotation(&rt, i, a.map_err_trace_exit_unwrap(1), with_text) - }) - .collect::>(); + .for_each(|(i, entry)| list_annotation(&rt, i, entry, with_text)); } }