-
Notifications
You must be signed in to change notification settings - Fork 7
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
edit: implement pazi edit #71
Conversation
src/main.rs
Outdated
@@ -186,6 +195,38 @@ fn _main() -> PaziResult { | |||
} | |||
} | |||
|
|||
if let Some(_) = flags.subcommand_matches("edit") { | |||
let mut fclone = frecency.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find an easy way to avoid this clone; feel free to try if you'd like.
I'm not too worried about performance in the pazi edit
path, so I didn't try too hard.
8a80df7
to
3a7dcda
Compare
Cargo.toml
Outdated
@@ -26,3 +26,6 @@ log = "~0.3" | |||
termion = "~1" | |||
chan = "~0.1" | |||
chan-signal = "~0.3" | |||
tempfile = "3" | |||
which = "2" | |||
sh_escape = { path = "./pkg/sh_escape" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You won't be able to publish the pazi crate to crates.io without specifying a version here, and also separately publishing sh_escape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, darn. Thanks.
I guess I'll have to think of an okay name for it so I can publish it separately then.
Let the caller decide to normalize as an optional second step
This is helpful for 'pazi edit'
To be used shortly in the 'pazi edit' work
This implements a 'pazi edit' subcommand which may be used to edit the pazi database in your $EDITOR of choice.
I fixed the pkg / crates.io issue by splitting out that code into https://github.com/euank/snailquote. At this point, I think this should be pretty much good to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
src/edit.rs
Outdated
continue; | ||
} | ||
|
||
line = line.trim_right(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Why not just
trim()
? - If you move this above line 135, it handles the case of lines with just whitespace instead of erroring on them.
And use iterators for that too.
@LinuxMercedes updated with the trim change, and I also shuffled around min/max per your out-of-band comment. I moved it to take an iterator and lazily do stuff itself |
Fixes #33
Note, this ends up doing significant work to expose the raw underlying frecency value to the user.
Unfortunately, that value is actually quite difficult to work with, but I think making a more human-editable version of it is something TBD independently and this is the most reasonable initial implementation.
This also does a decent chunk of work around escaping and editor-choosing, but so be it.