search

lightningdevkit / ldk-sample

Sample node implementation using LDK
Apache License 2.0
166 stars 94 forks source link

Support up/down arrows in CLI #48

Closed arik-so closed 1 year ago

arik-so commented 2 years ago

Perhaps use a library like https://github.com/mitsuhiko/dialoguer? It also adds support for autocompletion.

alecchendev commented 1 year ago

Hello! I'm consider making a PR adding this functionality sometime soon.

I saw in #50 it seems like this might be another thing that we wouldn't want to add a dependency for? I'm pretty new to contributing here, so I am actually not super certain on the overall mindset regarding dependencies. I'd imagine it's mainly because it's a security risk, and that it can add unnecessary complexity to a project? I'll plan on trying to make a PR without adding this dependency for now.

TheBlueMatt commented 1 year ago

ldk-sample is...a sample. Adding a lot of complexity to it to make it super duper user-friendly is an explicit non-goal - its code should stay simple to demonstrate how to build an LDK application, not add complexity to be super user-friendly. Both as a sample and as a demonstration of best-practices we also don't want to add dependencies unless we have a strong reason to believe we cannot reproduce the feature simply.

For the first reason I don't think we should bother with this at all, for the second we shouldn't add a dependency to do so.

alecchendev commented 1 year ago

Ah makes sense, thanks!