nom icon indicating copy to clipboard operation
nom copied to clipboard

What is expected behavior when selecting an item in "preview" mode?

Open larsks opened this issue 3 months ago • 10 comments

Running nom with the -f flag, it successfully displays items from the given RSS feed...but when selecting an item, it exits immediately. Is this expected behavior, or is this a bug? I've included a demo of the behavior here:

asciicast

larsks avatar Sep 29 '25 15:09 larsks

~~It sounds as if https://github.com/guyfedwards/nom/pull/140 might be related.~~

Never mind, I think that was a different issue.

larsks avatar Sep 30 '25 01:09 larsks

Just to clarify @guyfedwards I'm assuming this is not the expected behavior? Should it read the item and display it? I'm willing to work on a fix for this if that's the case.

GV14982 avatar Oct 12 '25 03:10 GV14982

Yea this is a bug. Selecting the item even in a preview feed should display

guyfedwards avatar Oct 12 '25 09:10 guyfedwards

@guyfedwards I had an idea for this, to just implement an InMemoryStore impl for the Store interface, and just use that for preview mode. Thoughts? I figured we could cut out some of the conditional logic around, because I'm pretty sure the issue here is that when it goes to fetch the item ID, there is no ID so it just exits.

GV14982 avatar Oct 12 '25 21:10 GV14982

Yea that's a really nice solution and much cleaner

guyfedwards avatar Oct 12 '25 22:10 guyfedwards

While I understand why using a sqlite in-memory database is an attractive solution, I would like to suggest instead writing a simple, pure-go in-memory store. Why? Because I am selfish and I am trying to make my life easier :smile:.

I have a fork that replaces sqlite with badger, primarily so that we can avoid the CGO dependency when building container images. With a pure go solution, the image doesn't need to include anything other than the nom binary; it doesn't require a full set of shared libraries, etc.

It's not ready for prime time (in particular, it doesn't include any facility for migrating from a sqlite store, which is why I haven't made a pr yet), but by keeping our sqlite dependency isolated in sqlitestore, we make this sort of thing easier to implement.

I have a alternative in-memory store implementation built on top of basic go data structures in https://github.com/larsks/nom/compare/feature/memory-store; it makes the follow changes:

  1. Move the sqlite based store to internal/store/sqlitestore.
  2. Add a new storage driver, internal/store/memorystore.

Note that this is built on top of some changes related to #167 (it doesn't depend on these changes, and I'd be happy to rebase things if folks are interested in this solution and want to avoid the GUID-based changes for now).

larsks avatar Oct 13 '25 17:10 larsks

Just because I keep having to go find it, this issue is attached to: https://github.com/guyfedwards/nom/pull/166

larsks avatar Oct 13 '25 17:10 larsks

That was what I had started working on, but I figured just reusing the existing code would be simpler (Which is why I just added a separate constructor that would handle creating an in memory SQLite store) 🙃

GV14982 avatar Oct 13 '25 17:10 GV14982

@larsks I'm not sure I see the benefit of moving to badger/another key/value store really. There are downsides of using CGO but I haven't seen them produce actual issues for users. If there was then I think looking at something like cznic/sqlite or ncruces/go-sqlite3 would be more likely, to maintain compatibility between versions. Either way, I don't think this is the place for such a large change discussion, I've created https://github.com/guyfedwards/nom/issues/168.

guyfedwards avatar Oct 13 '25 18:10 guyfedwards

Either way, I don't think this is the place for such a large change discussion, I've created https://github.com/guyfedwards/nom/issues/168.

Oh, I wasn't trying to have that discussion here. I was just introducing some background for why I was suggesting a non-SQLite based in-memory store.

larsks avatar Oct 13 '25 18:10 larsks