Skip to content
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

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

Closed
cmrschwarz opened this issue May 26, 2024 · 5 comments · Fixed by #129
Closed

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

cmrschwarz opened this issue May 26, 2024 · 5 comments · Fixed by #129
Assignees
Labels
annoying-stuff It's not a good experience it is this way

Comments

@cmrschwarz
Copy link
Contributor

cmrschwarz commented May 26, 2024

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

Any suggestion on which approach sounds best to implement?

@henriklovhaug henriklovhaug self-assigned this May 31, 2024
@henriklovhaug henriklovhaug added the annoying-stuff It's not a good experience it is this way label May 31, 2024
@henriklovhaug henriklovhaug moved this to In progress in MD-TUI May 31, 2024
@cmrschwarz
Copy link
Contributor Author

cmrschwarz commented May 31, 2024

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 henriklovhaug moved this from In progress to In review in MD-TUI Jun 1, 2024
@github-project-automation github-project-automation bot moved this from In review to Done in MD-TUI Jun 1, 2024
@henriklovhaug
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annoying-stuff It's not a good experience it is this way
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants