pimterry / notes

:pencil: Simple delightful note taking, with more unix and less lock-in.
https://github.com/pimterry/notes
MIT License
1.24k stars 82 forks source link

Interactive Mode #17

Open khamer opened 7 years ago

khamer commented 7 years ago

I'm willing to put together a pull request for interactive mode, but I'd most likely want to depend on junegunn/fzf for the interactive functionality. @pimterry - does that jive with what you were looking to do?

pimterry commented 7 years ago

I don't know how distribution is going to work easily if we start bundling in extra packages - I have a very strong preference for being able to pull down and get started in a single command on any system with a standard shell available, and I worry that this would start to break that.

I'm also very cautious about letting the core of notes itself get too large, rather than being a set of small dumb commands. I want a toolkit of components, not a big blob that does everything.

Is there a practical way this could be layer on top of the command line notes interface? What commands would notes have to provide to make it possible to build a UI separate that called into it to provide this interface? We already just build an autocomplete-style dropdown by just calling notes find with the provided input, I think. Does that sound like a potentially practical direction for what you're imagining?

khamer commented 7 years ago

Something like

$ editor $NOTES_DIRECTORY/$(notes find | fzf)

Does what I'm suggesting. I can dig a little bit to see what it'd take to simplify this. Would you rather me investigate dmenu than fzf? That seems like another viable option to provide interactive mode, and I've done a fair amount of scripting with both.

primis commented 7 years ago

We could create a new command, one called "notes" and the other called "vnotes" or something, and vnotes would be visual mode. This would solve the "blob" problem quite well.

pimterry commented 7 years ago

@primis Yeah, a separate command that wraps notes feels like a good place to start.

@khamer:

I have no preference - build it with whatever you think works best!

Just to focus on your example for a second: in general, we should be avoiding this tool understanding notes internals too much, and it should instead be making calls to the built-in commands. As a case in point, that example using $NOTES_DIRECTORY is going to break if we allow setting the notes directory in config (#21), and that example also doesn't know the default notes directory to use if the env var isn't set. I know it's only an illustrative example, but just something to watch out for.

That doesn't necessarily mean you can't start with an implementation that knows about those internal details, but it'd be good to keep a note of what information it has to duplicate, so we can eventually expand the built-in notes commands so that that's no longer necessary (in the above example: by completing #23, so you can get the full note paths from notes directly)

nosarthur commented 2 years ago

I have notes 1.0.0 and vim 8.2.3025

This command works where n is an alias to notes

n o `n ls |fzf`

But this one doesn't work completely

n ls | fzf | n o

There is a warning

Vim: Warning: Input is not from a terminal

The correct file is still opened. But after exiting vim, my bash is messed up: terminal input doesn't show up. Why the 2nd form doesn't work?

pimterry commented 2 years ago

Thanks for the info @nosarthur. I've just done some testing, and it seems this was a bug that was introduced in 1.0.0 by this change: https://github.com/pimterry/notes/commit/15f870ed4823f8b217786d8debd3ee62fb31b05e.

That was added to fix GitHub Actions, where /dev/tty is not available. It's actually buggy though, it's using the wrong test, so it effectively disables the </dev/tty part for all cases, which means in some piped cases, as you're seeing, you hit issues.

I think I've fixed this in https://github.com/pimterry/notes/commit/6315323578ca8812e254ce0cd63ac12deb4b69a3. Can you test that out and let me know?

nosarthur commented 2 years ago

Thanks @pimterry The new commit 6315323 works

pimterry commented 2 years ago

Great! Thanks @nosarthur, that fix is now released as v1.0.1.