feat: Add spawn vim docs #651

deepanchal · 2024-07-01T02:43:12Z

Description

This PR adds how-to-spawn-vim documentation under Recipes > Applications > Spawn External Editor (Vim)

I am now using hello-world-template as base for my example to keep things simple (31fc71c) and I have added logic to spawn vim in a separate commit (8380d5b).

I am not familiar with astro but I tried to use other examples as reference to write this documentation. Please let me know if I missed anything. Thanks!!

Related Issue

Ref #406

Images

joshka · 2024-07-01T03:25:03Z

This seems overly specific to the component template and has a lot of code just to show off the vim part. I have a few concerns with this:

we have to maintain that code - this is likely to get stale fast
the specific problem and solution are obscured by everything else that is part of the application (actions, events, keys, etc.)
people unfamiliar with the component template have to learn how that works just to understand the recipe
The tui module setup code likely will go away at some point as it gets replaced by stuff from feat(backend): Add convenience methods for setting up terminals ratatui#1180 or feat: simplify backend and terminal creation ratatui#280 (or whatever we settle on)
What the problem is and how the solution solves it isn't all that clear

If you were able to simplify this to an app that just showed off the problem and solution, this would be much clearer.
The problem are:

applications read events, either synchronously, in another thread, or in an async task via the event stream
applications manage the terminal state, such as raw mode, alternate screen, mouse capture etc.

The solution really needs to show off how to:

before spawning processes suspend / stop any event reading
restore the terminal state up that whatever your sub process needs to work properly
reset these after the sub process finishes
any abstractions like the component template on top of the plain crossterm stuff should probably be omitted (or perhaps noted for follow up with a some small snippets)

This likely should also have a bit of discussion about termion at the end that shows how to access the backend to do this (requires an unstable flag to get at the backend_mut function and call the suspend functions)

This is just my opinion - I know you've been working on this with @kdheepak who knows a lot more about this template than I do, so I'd let him chime in about whether this is worth keeping in this form or simplifying.

deepanchal · 2024-07-01T03:37:05Z

Thanks for the feedback!
As I was writing this, I also noticed that there's quite a lot of boilerplate coming from component template. I was in the process of removing excess code from this template. I totally agree with your key points on clear problem and solution. I will address those points and make it more cleaner. Also, I am not familiar with writing documentation so I am sure that the wording in the documentation can be improved. I will try to improve docs as well. I will wait for @kdheepak's opinion before I make any big changes in the PR.

kdheepak · 2024-07-01T05:28:48Z

I agree with your comments. It should be a lot simpler to discuss just the aspect of starting an external process like vim.

One thing I wanted to note is that when I've attempted to do do this without the pattern used in the component template, it resulted in issues where ANSI RGB values would be printed into the TUI on returning back from vim.

See for example this commit from @orhun where he made a change to ensure it worked correctly: orhun/rattler-build@84ea16a

I first saw this issue in kdheepak/taskwarrior-tui#46
The reason this occurred is because when vim wants to find out the background color of the terminal, and the response to that request is written to stdin.

When this issue occurs, the response is written to stdin which is read by crossterm as if a user is typing it and that is sent to the keyevents and that ends up getting processed by the TUI.

I know that the combination of using select! + cancellation_token + tokio the way it is used in the component template does not cause this issue. But I haven't tested other ways to narrow down exactly what causes it and what doesn't cause it.

This issue is particularly annoying because it is not always reproducible. I never noticed it on iTerm until I decreased the tick time when using tokio. I noticed it happened more when I was using a background thread to send events to the main thread and was not able to solve it when using a AtomicBool as a Mutex lock to pause the background thread.

Currently, I don't know why the other methods didn't work, and I definitely didn't comprehensively test everything. I was changing multiple things at the same time when I was testing this in taskwarrior-tui and landed on a solution with tokio, select! and cancellation_tokens that worked.

I would like to ensure that this RGB issue doesn't occur with whatever recipe is posted on ratatui.rs because if it does we'll get a lot of questions about what is going on for users that copy paste the code.

kdheepak · 2024-07-01T05:29:59Z

Thanks for sharing a PR and describing your steps to solve this problem here and on discourse! Even just this information will be useful for those that try to do the same thing and come across this!

deepanchal · 2024-07-01T16:46:00Z

@joshka @kdheepak
I have force pushed to this branch with a much simpler example using hello-world template and updated the docs accordingly. I just had a few questions

I feel that I am not explaining the process clearly in Handle Events section. Is the template too simple that it doesn't address the problems and solutions mentioned in feat: Add spawn vim docs #651 (comment)
Does this example need vhs gif to demonstrate how it works?
I have added a tip to use edit crate for opening editor at the very bottom. Should this be moved to Handle Events section?
Should I include my original diff from this pr comment Add section on how to spawn vim from a ratatui application #406 (comment) as a tip in this doc?

If any of you can review these new changes whenever you get a chance, that will be helpful! Thanks!!

code/how-to-spawn-vim/src/main.rs

joshka

The goal of the recipes section is less step by step tutorial and more an explanation of a snippet of code. What about dropping this down to just two code blocks, one showing the the main, handle_events and run_editor code in a single block, with the main and handle events methods collapsed, and when showing the full file, fully collapsed?, E.g. use the following and add a direct link to the code file in the repo.

```rust collapsed title="main.rs (click to expand)"

The main thing this article really needs to do is say something like (feel free to rephrase):

Before you run another app from your app, your app is in control of the input and output. Your app needs to doing that while the other app is running otherwise ...

It likely also needs to draw comparisons about synchronous, threaded, and async event handling.

joshka · 2024-07-02T17:06:50Z

code/how-to-spawn-vim/src/main.rs

+                                stdout().execute(LeaveAlternateScreen)?;
+                                disable_raw_mode()?;
+                                // Launch vim and wait for status
+                                // You may change this to other editors like nvim, nano, etc.
+                                Command::new("vim").arg("/tmp/a.txt").status()?;
+                                stdout().execute(EnterAlternateScreen)?;
+                                enable_raw_mode()?;
+                                // Clear the terminal and force a full redraw on the next draw call.
+                                terminal.clear()?;


This is really the crux of the entire article, and it deserves it's own code block, everything else is boilerplate that could be in any app. I'd suggest pulling this out into a function and showing it with some narrative.

I agree that this part is important, but it is the obvious part imo, and pausing the key event handler before spawning vim and restarting the key event handler after resuming from vim exiting is imo the part that will trip most people.

In these examples we don't have a background key handler, so what I expect will happen is that someone will copy this code into their app and it won't work as expected.

I apologize for the delay in addressing this PR over the past two weeks. I agree with @kdheepak's concern about users with advanced setups experiencing issues when copying the snippet. Here are two possible solutions:

Add a note to the documentation illustrating how to spawn an external process (e.g., Vim) using a component template. I have created a proof of concept here: feat: Add ExecuteCommand action in component template templates#62

Enhance our snippet for spawning Vim by utilizing a similar but simplified version of the tui module component template.

src/content/docs/recipes/apps/index.md

code/how-to-spawn-vim/src/main.rs

…d-tutorial, update root lockfile Signed-off-by: Deep Panchal <[email protected]>

Signed-off-by: Deep Panchal <[email protected]>

…, update anchors Signed-off-by: Deep Panchal <[email protected]>

…tandable Signed-off-by: Deep Panchal <[email protected]>

… fix md link in index.md Signed-off-by: Deep Panchal <[email protected]>

deepanchal · 2024-07-13T19:53:49Z

I have updated the PR to focus more on the logic for spawning Vim. I have also updated the image of generated docs on PR description. Please let me know if this is acceptable :)
Thanks!

orhun

LGTM overall, but I feel like it is missing some parts.

Mainly, the edge case of ANSI code being printed should be mentioned somewhere. We can also give some example TUI apps which implements this.

Also, if the user only wants to launch vim to edit something, using e.g. edtui might also come in handy.

Lastly, clippy is complaining :)

orhun · 2024-07-26T07:14:25Z

src/content/docs/recipes/apps/spawn-vim.md

+events. The `handle_events` function listens for key events and returns an `Action` based on the key
+pressed.
+
+## Spawning vim


nit:

Suggested change

## Spawning vim

## Spawning Vim

orhun · 2024-07-26T07:14:35Z

src/content/docs/recipes/apps/spawn-vim.md

+
+## Spawning vim
+
+Now, lets define the function `run_editor` function attached to `Action::EditFile` action.


Suggested change

Now, lets define the function `run_editor` function attached to `Action::EditFile` action.

Now, let's define the function `run_editor` function attached to `Action::EditFile` action.

orhun · 2024-07-26T07:15:54Z

src/content/docs/recipes/apps/spawn-vim.md

+[full code](https://github.com/ratatui-org/ratatui-website/tree/main/code/how-to-spawn-vim/src/main.rs).
+Next, we spawn a child process with `Command::new("vim").arg("/tmp/a.txt").status()` which launches
+`vim` to edit the given file. At this point, we have given up control of our TUI app to vim. Our TUI
+app will now wait for the exit status of the child process (i.e., Vim). Once the user exits Vim, our


I feel like Vim is mentioned too many times here :D

Suggested change

app will now wait for the exit status of the child process (i.e., Vim). Once the user exits Vim, our

app will now wait for the exit status of the child process. Once the user exits Vim, our

deepanchal marked this pull request as ready for review July 1, 2024 02:47

deepanchal mentioned this pull request Jul 1, 2024

Add section on how to spawn vim from a ratatui application #406

Open

deepanchal force-pushed the feat/how-to-spawn-vim-example branch from 212c8ca to 883d87b Compare July 1, 2024 16:15

deepanchal commented Jul 1, 2024

View reviewed changes

code/how-to-spawn-vim/src/main.rs Outdated Show resolved Hide resolved

deepanchal mentioned this pull request Jul 1, 2024

fix: rename Action::Refresh -> Action::ClearScreen, add handler, update tui restore logic after suspend ratatui/templates#61

Merged

joshka reviewed Jul 2, 2024

View reviewed changes

deepanchal added 5 commits July 13, 2024 09:49

feat(code): add how-to-spawn-vim example starter code from hello-worl…

da05cc0

…d-tutorial, update root lockfile Signed-off-by: Deep Panchal <[email protected]>

feat(code): add logic to spawn vim to how-to-spawn-vim example

a115d84

Signed-off-by: Deep Panchal <[email protected]>

fix(code): update & add anchors in how-to-spawn-vim example

c6da4d0

Signed-off-by: Deep Panchal <[email protected]>

feat(docs): add spawn-vim docs in recipes/apps section

bd76f0e

Signed-off-by: Deep Panchal <[email protected]>

fix(code): use terminal.clear() to refresh terminal after exiting vim…

08c1a91

…, update anchors Signed-off-by: Deep Panchal <[email protected]>

deepanchal mentioned this pull request Jul 13, 2024

feat: Add ExecuteCommand action in component template ratatui/templates#62

Open

deepanchal added 2 commits July 13, 2024 14:41

refactor(code): update how-to-spawn-vim code to be modular and unders…

5eaf697

…tandable Signed-off-by: Deep Panchal <[email protected]>

feat(docs): improve spawn-vim docs by emphasizing spawning vim logic,…

7be0016

… fix md link in index.md Signed-off-by: Deep Panchal <[email protected]>

deepanchal force-pushed the feat/how-to-spawn-vim-example branch from 5db9906 to 7be0016 Compare July 13, 2024 19:44

deepanchal requested review from joshka and kdheepak July 13, 2024 19:54

orhun reviewed Jul 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add spawn vim docs #651

feat: Add spawn vim docs #651

deepanchal commented Jul 1, 2024 •

edited

Loading

joshka commented Jul 1, 2024

deepanchal commented Jul 1, 2024

kdheepak commented Jul 1, 2024 •

edited

Loading

kdheepak commented Jul 1, 2024 •

edited

Loading

deepanchal commented Jul 1, 2024

joshka left a comment

joshka Jul 2, 2024

kdheepak Jul 2, 2024

kdheepak Jul 2, 2024

deepanchal Jul 13, 2024

deepanchal commented Jul 13, 2024

orhun left a comment

orhun Jul 26, 2024

orhun Jul 26, 2024

orhun Jul 26, 2024


		## Spawning vim

		Now, lets define the function `run_editor` function attached to `Action::EditFile` action.

	Now, lets define the function `run_editor` function attached to `Action::EditFile` action.
	Now, let's define the function `run_editor` function attached to `Action::EditFile` action.

	app will now wait for the exit status of the child process (i.e., Vim). Once the user exits Vim, our
	app will now wait for the exit status of the child process. Once the user exits Vim, our

feat: Add spawn vim docs #651

Are you sure you want to change the base?

feat: Add spawn vim docs #651

Conversation

deepanchal commented Jul 1, 2024 • edited Loading

Description

Related Issue

Images

joshka commented Jul 1, 2024

deepanchal commented Jul 1, 2024

kdheepak commented Jul 1, 2024 • edited Loading

kdheepak commented Jul 1, 2024 • edited Loading

deepanchal commented Jul 1, 2024

joshka left a comment

Choose a reason for hiding this comment

joshka Jul 2, 2024

Choose a reason for hiding this comment

kdheepak Jul 2, 2024

Choose a reason for hiding this comment

kdheepak Jul 2, 2024

Choose a reason for hiding this comment

deepanchal Jul 13, 2024

Choose a reason for hiding this comment

deepanchal commented Jul 13, 2024

orhun left a comment

Choose a reason for hiding this comment

orhun Jul 26, 2024

Choose a reason for hiding this comment

orhun Jul 26, 2024

Choose a reason for hiding this comment

orhun Jul 26, 2024

Choose a reason for hiding this comment

deepanchal commented Jul 1, 2024 •

edited

Loading

kdheepak commented Jul 1, 2024 •

edited

Loading

kdheepak commented Jul 1, 2024 •

edited

Loading