henriklovhaug / md-tui

Markdown renderer in the terminal
GNU Affero General Public License v3.0
208 stars 13 forks source link

Opening mdt in a large directory freezes the terminal. #127

Closed cmrschwarz closed 5 months ago

cmrschwarz commented 6 months ago

Observed Behavior

When opening mdt in a large directory (e.g. /), it displays a LOADING... message and becomes unresponsive (even to keyboard events like q).

Expected Behavior

Mdt should not become unresponsive, and close immediately when q is pressed.

Open Questions

  1. Instead of displaying a loading screen, it might be nicer (although more work to implement) to immediately start, but lazily populate the list of documents. It would probably be a good idea to use a breadth first search for this, so that the search does not get 'stuck' in some deeply nested directory right away.

  2. Another option would be to display a folder explorer instead of a recursive search.

  3. Should mdt respect Ctrl+C as a termination request? It currently doesn't, but maybe should (at least within the loading screen, if we keep that).

henriklovhaug commented 6 months ago

On vacation. Won't start soon. However, to solve this, it would require another thread to read the directory and sending it to the UI thread. Or going full asynchronous. It's currently fully sync on one thread (not really, but for this it is). Both sounds like pain. Ctrl + C or other signal should be respected tho. Just haven't bothered dealing with them yet.

cmrschwarz commented 6 months ago

All good, this is OSS, not a job! Once you had to pkill mdt a few times you just know not to do that ;). Just thought its a good idea to have this issue pointed out here. Wishing you a pleasant vacation.

henriklovhaug commented 5 months ago

Any suggestion on which approach sounds best to implement?

cmrschwarz commented 5 months ago

Seems like a matter of preference. For my usecases, I think I would slightly prefer the folder browser. I usually have a meaningful folder structure and having all files displayed at once is more overwhelming than convenient. It would also allow mdt to remain single threaded, which is a nice property for cli tools to have. (I guess it's possible to do the search single threaded too using some kind of lazy generator, eh.). But I would be happy either way. I can imagine you went with the recursive search initially because it best fits your workflow, so please don't feel pressured to change that.

henriklovhaug commented 5 months ago

It's now fixed. Files gets accessible as it finds them. Some few hiccups I didn't patch this version related to how they are sorted, but yeah, no more loading screen