Open pane scrollback in default editor

[imsnif]

The experience should be:

1. Press a Zellij shortcut (something like ctrl+s + e)
2. Have the current pane replaced with your default editor, opened to a temporary file containing all the scrollback of the terminal pane and scrolled to the same line
3. Search, save, copy/paste, do whatever you want
4. When you quit the editor (or press ctrl+s + e again), the original pane comes back

Depends on: #1375

[imsnif]

@cosminadrianpopescu - you mentioned you might like to work on this. I wanted to open this as a separate issue because it's part of our roadmap.

[cosminadrianpopescu]

Yes, I would like to work on this.

[cosminadrianpopescu]

How would I go about it? What is the idea? How do I replace the content of the pane with a command without putting that command in the history of the current pane's shell?

Also, when running the command, is there something to know about it regarding different operating systems?

My idea would be to create a new temporary pane which would replace the current one. Is there a better approach? If this is the right approach, how do I know when this temporary panel is closed, to react and put back the old pane?

Any hints?

[imsnif]

I think the basic approach should be:

1. When we receive the EditScrollback action (which we should create), we first dump the scrollback of the current terminal pane into a temporary file. This is something we do in the screen_thread, in Tab in a similar function to the one you already know here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/tab/mod.rs#L1385
2. Then, we need to open a pane with the default editor and point it to that file. In our pty_thread we already have a facility for that. So if we send it the right instruction (PtyInstruction::SpawnTerminal with the TerminalAction::OpenFile(path) parameter), it'll do all this work for us. Here's an example where our plugins send it this instruction:

   zellij/zellij-server/src/wasm_vm.rs

   Lines 395 to 399 in c53b323

   	.send_to_pty(PtyInstruction::SpawnTerminal(
   	Some(TerminalAction::OpenFile(path)),
   	ClientOrTabIndex::TabIndex(plugin_env.tab_index),
   	))
   	.unwrap();

3. The above function will send back a message to Screen (and eventually to the relevant Tab) once the pane is open, giving us the pane ID. The problem is that it'll send the message to us as a new pane and we'll open it as a new pane instead of replacing it with the current pane.
4. What I suggest we do is create a new sort of PtyInstruction instead of PtyInstruction::SpawnTerminal (https://github.com/zellij-org/zellij/blob/main/zellij-server/src/pty.rs#L85). Maybe something like PtyInstruction::OpenInplaceEditor which will act in the same way as PtyInstruction::SpawnTerminal, but carry with it a bit of extra context that we can use in order to identify the pane we're operating on. We could use the temporary file name to where we wrote the scrollback in order to identify it (I'm counting on these being unique throughout the system).
5. So we send the new OpenInplaceEditor instruction to the pty_thread, and we keep a temporary filename on our Tab state... maybe in a HashMap<file_name, original_pane_id> for later. The pty_thread sends us a new kind of ScreenInstruction back (instead of ScreenInstruction::NewPane - https://github.com/zellij-org/zellij/blob/main/zellij-server/src/pty.rs#L89) - maybe something like ScreenInstruction::OpenInplaceEditor which will contain everything NewPane contains, but also the temporary file so we can identify it in Tab.
6. Once that's done, in Screen we handle this new instruction, thread it down to the relevant Tab which will then:

• replace the pane's geom with the geom of the pane we're replacing
• place the pane we're replacing in self.panes_to_hide (this is the strategy we use for placing a pane in full screen - so we can safely reuse it here)
• render everything and we're done

7. For closing the pane, we'll need to keep another bit of state on Tab, maybe a HashMap<scrollback_pane_id, originating_pane_id>, so that we know to take the pane our of self.panes_to_hide instead of going through our whole normal close_pane jam.

Makes sense? Shall I go into more details about specific sections? A lot of the questions you asked are already handled by the app itself as you can see above. It's just a question of knowing where to look. And since this is a bit of an involved (and in need of refactoring) app, I'm happy to guide you as closely as you'd like me to :)

[cosminadrianpopescu]

Ok. I will have a look and see what I can come up to. I will ask more specific questions when I will arrive at a certain point that I can't handle myself.

Thank you.

[imsnif]

Hey @cosminadrianpopescu - sorry to press on this, I just want to release this feature with the next release in ~1 week. Do you think you can see it to completion, or are you too busy?

[cosminadrianpopescu]

I am planning to work on this from Wednesday. I hope to finish it until Sunday. I will keep you updated.

I looked already at it. It seems straight forward what you described. What I'm most worried is sending those information back and forth when I want to see what panel is opened (points 5 and 7 in your tutorial). I remember the difficulties I've had in the test case of the editor dump feature. Other than this, it should be straight forward and I should be able to finish it until Sunday.

[imsnif]

Sounds great, thanks for the quick response!

If you have trouble, I'm also happy to work on this together. You can also give me a ping on Discord if you like.

[cosminadrianpopescu]

I am at step 6 and having some issues. So far, what I've come up with is this:

            ScreenInstruction::OpenInPlaceEditor(pid, file_name, client_id) => {
                if let Some(active_tab) = screen.get_active_tab_mut(client_id) {
                    if let Some(pane1) = active_tab.get_active_pane(client_id) {
                        scrollbacks.insert(file_name, pane1.pid());
                        let geom = pane1.current_geom().clone();
                        active_tab.new_pane(pid, Some(client_id));
                        if let Some(pane2) = active_tab.get_active_pane_mut(client_id) {
                            pane2.set_geom(geom);
                        }
                        else {
                            log::error!("New editor pane not found. Maybe there is an issue with launching it?: {:?}", client_id);
                        }
                    }
                } else {
                    log::error!("Active tab not found for client id: {:?}", client_id);
                }
                screen
                    .bus
                    .senders
                    .send_to_server(ServerInstruction::UnblockInputThread)
                    .unwrap();
                screen.update_tabs();

                screen.render();
            }

I have 2 issues:

1. The panel is hidden and replaced with the editor buffer, but the geometry is not right. The editor panel has only half of the screen, like it would have if the original buffer would also be displayed.
2. I see that the self.panes_to_hide is private in tiled_panes. Since I am in screen, how do I reach that? My approach is to try to create some public function in tiled_panes and another one in tab.

[imsnif]

1. Does its frame also take up half the screen? Or does the frame fill up the whole space but just the editor itself takes up half the screen?

If it's the first option, it's probably that we need to update the pty about the size change. Like we do in this macro: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/tab/mod.rs#L47-L59 (though it will probably be best to do this as part of the original message exchange, since in this case we already know the size - will save a roundtrip).

If it's the second option, my guess is that the updated geom is somewhat wrong. If you log it (eg. with log::info or your favorite debugging method), does it show the half or the full geometry?

2. Yeah, that's what I would do.

[cosminadrianpopescu]

Does its frame also take up half the screen? Or does the frame fill up the whole space but just the editor itself takes up half the screen?

The frame fills up the whole space, but just the editor takes up half.

The issues is that the geometry is always the same. So:

            ScreenInstruction::OpenInPlaceEditor(pid, file_name, client_id) => {
                if let Some(active_tab) = screen.get_active_tab_mut(client_id) {
                    if let Some(pane1) = active_tab.get_active_pane(client_id) {
                        scrollbacks.insert(file_name, pane1.pid());
                        let geom = pane1.current_geom().clone();
                        log::error!("GEOM is: {:?}, {:?}, {:?}, {:?}", geom.x, geom.y, geom.rows.inner, geom.cols.inner);
                        active_tab.new_pane(pid, Some(client_id));
                        if let Some(pane2) = active_tab.get_active_pane_mut(client_id) {
                            log::error!("NOW GEOM is: {:?}, {:?}, {:?}, {:?}", geom.x, geom.y, geom.rows.inner, geom.cols.inner);
                            pane2.set_geom(geom);
                        }
                        else {
                            log::error!("NOT Found pane 2 : {:?}", pid);
                        }
                    }
                } else {
                    log::error!("Active tab not found for client id: {:?}", client_id);
                }
                screen
                    .bus
                    .senders
                    .send_to_server(ServerInstruction::UnblockInputThread)
                    .unwrap();
                screen.update_tabs();

                screen.render();

So, if I do this, I always get the same geometry:

ERROR  |zellij_server::screen    | 2022-05-31 20:19:12.313 [screen    ] [zellij-server/src/screen.rs:881]: GEOM is: 0, 1, 53, 200 
ERROR  |zellij_server::screen    | 2022-05-31 20:19:12.315 [screen    ] [zellij-server/src/screen.rs:884]: NOW GEOM is: 0, 1, 53, 200 

What is even stranger is that if I do this:

            ScreenInstruction::OpenInPlaceEditor(pid, file_name, client_id) => {
                if let Some(active_tab) = screen.get_active_tab_mut(client_id) {
                    if let Some(pane1) = active_tab.get_active_pane(client_id) {
                        scrollbacks.insert(file_name, pane1.pid());
                        let geom = pane1.current_geom().clone();
                        log::error!("GEOM is: {:?}, {:?}, {:?}, {:?}", geom.x, geom.y, geom.rows.inner, geom.cols.inner);
                        else {
                            log::error!("NOT Found pane 2 : {:?}", pid);
                        }
                    }
                } else {
                    log::error!("Active tab not found for client id: {:?}", client_id);
                }
                screen
                    .bus
                    .senders
                    .send_to_server(ServerInstruction::UnblockInputThread)
                    .unwrap();
                screen.update_tabs();

                screen.render();

You notice that I'm not even opening the second pane. The geometry is still the same:

ERROR  |zellij_server::screen    | 2022-06-01 10:31:15.727 [screen    ] [zellij-server/src/screen.rs:881]: GEOM is: 0, 1, 53, 200 

So, I'm a little bit confused.

[imsnif]

My guess then is that this is a case of the editor not knowing what size it should take up. Not sure why it takes up exactly half, but we essentially need to make sure a SIGWINCH is sent to it.

Have you tried using the resize_pty macro I pointed out? For a quick test you can also try resizing the pane and seeing if it fixes the problem.

[cosminadrianpopescu]

I'm trying to use resize_pty, but I don't have an os_api in screen. How can I get an os_api in screen?

[imsnif]

You can use this unwieldy method: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/screen.rs#L598
That's how Screen gives its os_api to Tab - so should work well here.

[cosminadrianpopescu]

Trying this and I'm getting again issues with borrowing variables:

error[E0502]: cannot borrow `screen.bus.os_input` as immutable because it is also borrowed as mutable
   --> zellij-server/src/screen.rs:900:48
    |
28  |                 $pane.get_content_columns() as u16,
    |                 --------------------------- mutable borrow later used here
...
891 |                 if let Some(active_tab) = screen.get_active_tab_mut(client_id) {
    |                                           ------------------------------------ mutable borrow occurs here
...
900 |                             resize_pty!(pane2, screen.bus.os_input.as_ref().unwrap().clone());
    |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here
                                                                                                                                                                               
For more information about this error, try `rustc --explain E0502`.
error: could not compile `zellij-server` due to previous error

[cosminadrianpopescu]

Finally fixed and now it's fine. :) The panel has the proper size.

So, I will continue with the next steps. Thank you.

[cosminadrianpopescu]

So, now for point 7:

For closing the pane, we'll need to keep another bit of state on Tab, maybe a HashMap<scrollback_pane_id, originating_pane_id>, so that we know to take the pane our of self.panes_to_hide instead of going through our whole normal close_pane jam.

How do I know when the panel is closed?

[imsnif]

When a pane is closed (either through a Zellij shortcut or by the program running inside the pane itself) it eventually bubbles down to here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/tab/mod.rs#L1348

[cosminadrianpopescu]

For floating panes it seems to work perfectly.

However, there are multiple issues with the geometry when it comes to tiled panes. First approach was to save the original geometry and then to restore it in close_pane. This works perfectly for one pane. However, when I have multiple panes, this is what happens.

Initially, I get this layout.

+------------------+------------------+
|                                     |
|                                     |
|                                     |
|                Pane 1               |
|                                     |
|                                     |
|                                     |
+------------------+------------------+
|                                     |
|                                     |
|                                     |
|                Pane 2               |
|                                     |
|                                     |
|                                     |
+------------------+------------------+

Then I select the second pane (Pane 2). There I do EditScrollback. The second pane is propely replaced with the editor. But the first panel, is also loosing the space that would've normally been occupied by the new pane (with the editor) which has the geometry of Pane 2.

+------------------+------------------+
|                  |                  |
|                  |                  |
|                  |                  |
|      Pane 1      | Empty space      |
|                  |                  |
|                  |                  |
|                  |                  |
+------------------+------------------+
|                                     |
|                                     |
|                                     |
|         Scroll edited in editor     |
|                                     |
|                                     |
|                                     |
+------------------+------------------+

Im am thinking of using some strategy like when a panels is toggled as floating. But even then, probably that if I close the pane opening, then the Pane 1 would get modified and it would take all the space. I think that we should not use the normal new_pane function, but instead a new one which would not process the geometry. And I see here that the geometry is supposed to happen later. Where is this?

Also, can we create a new pane without any geometry?

Another strategy that I was thinking about was to try to somehow put it in the floating_panes, then give it the geometry of the panel being replaced. Like this, normally the geometry would not be modified, right? But then, what are the implications of such a thing.

Any ideas?

[imsnif]

I agree we should not be using the new_pane method. The geometry is set in the tiled_panes.insert_pane method here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/panes/tiled_panes/mod.rs#L126

The insert_pane method basically means "Find a room for this pane and adjust the other panes accordingly". Maybe its name is not super clear.

I think we need to use a new method in Tab instead of new_pane as you suggest that would switch the geometries, and also a new method in tiled_panes that would add the new pane to its state without changing the geometry. (and also we need to make sure the original pane is in panes_to_hide so it won't be rendered twice - but iirc you already took care of that).

[cosminadrianpopescu]

I agree we should not be using the new_pane method. The geometry is set in the tiled_panes.insert_pane method here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/panes/tiled_panes/mod.rs#L126

I'll look into it.

and also a new method in tiled_panes that would add the new pane to its state without changing the geometry

This new method should replace which call?

and also we need to make sure the original pane is in panes_to_hide so it won't be rendered twice - but iirc you already took care of that

Yes, I took care of this. As I was saying, for floating panes it seems to be working without issues. I imagine that this is because when we have floating panes, inserting a new pane does not impact the geometry of others.

[imsnif]

This new method should replace which call?

The tiled_panes.insert_pane call.

[cosminadrianpopescu]

So far, I have this:

    pub fn create_pane(&mut self, pid: PaneId, client_id: Option<ClientId>, geom: PaneGeom) {
        if self.floating_panes.panes_are_visible() {
            self.new_pane(pid, client_id);
        } else {
            if self.tiled_panes.fullscreen_is_active() {
                self.tiled_panes.unset_fullscreen();
            }
            if let PaneId::Terminal(term_pid) = pid {
                let next_terminal_position = self.get_next_terminal_position();
                let new_terminal = TerminalPane::new(
                    term_pid,
                    geom, // the initial size will be set later
                    self.style,
                    next_terminal_position,
                    String::new(),
                    self.link_handler.clone(),
                    self.character_cell_size.clone(),
                    self.terminal_emulator_colors.clone(),
                    );
                self.tiled_panes.add_pane_with_existing_geom(pid, Box::new(new_terminal));
                // self.tiled_panes.insert_pane(pid, Box::new(new_terminal));
                // self.should_clear_display_before_rendering = true;
                if let Some(client_id) = client_id {
                    self.tiled_panes.focus_pane(pid, client_id);
                }
            }
        }
    }

This works better. There are however still issues: some crashes that I need to
investigate. Also, any idea why the frame is not drawn?

[cosminadrianpopescu]

Uff. This has other issues. If I create a pane down, I switch to the panel on the bottom, I replace with the editor, then I switch to the first panel (on top) and close it, the geometry of the panel with the buffer being edited is messed up.

[imsnif]

Want to upload the code to a fork somewhere and I'll take a look? (granted though, I need to leave the house in ~1 hour, so might only get to it tomorrow)

[cosminadrianpopescu]

So, what I would expect would be this:

tail -f /tmp/file

The output of the pane is this:

file content line 1

Then, I launch EditScrollback. A vim editor opens with file content line 1.

Then, I add another pane and do echo "file content line 2" >> /tmp/file

Then I go back to the pane where the buffer is edited and I close the editor. I would expect now to see on the screen

file content line 1
file content line 2

But I only see the first line. More, then if I go again to a new pane and do `echo "file content line 3" >> /tmp/file, I would expect to see on the screen

file content line 1
file content line 2
file content line 3

But what I actually see is

file content line 1
file content line 3

that is actually wrong.

So, do you think that my strategy could solve this issue?

[cosminadrianpopescu]

I had another 2 commits, one reverting the previous one and another one implementing the strategy I mentioned. It works perfectly now for tiled_panes (meaning it's not losing any content) but for floating panes only issue 1 is solved (you can't scroll through the pane). The second issue is not yet solved, althought I don't understand why. If you open a floating pane, edit the buffer and then you embed it and then you close it, it will not be replaced by the old panel. The old panel you can retrieve it by showing again the floating panes. What am I missing? Because normally, the pane being already in the tiled_panes and getting the geometry of the editor pane, it should be displayed in it's place, right?

[imsnif]

Hey @cosminadrianpopescu - sorry for not responding, fell asleep quite early.
I'll take a look at all of those issues today and bring this to the finishing line.

I know you're busy today, but if I could grab just a few moments on your time to help me out, could you:

1. Open a draft PR to the main repo so that you'll get credit for these changes? (you did most of the work after all!)
2. I know the reason my strategy with the suppressed_panes was losing content and it's an easy fix. Other than that, are there other issues there?

[cosminadrianpopescu]

Found the problem. Although the pane is embedded, the self.floating_panes.move_clients_out_of_pane(id) still returns true. So, is this a bug? Because if not, then the behaviour is completely normal. It means that this is how zellij is supposed to behave.

[cosminadrianpopescu]

I know the reason my strategy with the suppressed_panes was losing content and it's an easy fix. Other than that, are there other issues there?

So, what do you advise? I should go back to commit c52a203a20cf4c87c147be8b9c193ed6458c1038 to put back your strategy? Because right now, I put in place the other strategy.

I will open the PR.

[cosminadrianpopescu]

But before opening the PR, let me know how do you want me to open? Where it is now or at the commit c52a203a20cf4c87c147be8b9c193ed6458c1038?

[imsnif]

Open it where it is now. I'll take a look at this strategy and only change it if there's a need. I'm a little worried that it's a bit complex (I know I'll forget why we did this in 2 months :) ) - but let's see.

[imsnif]

Just pushed my fixes from yesterday. These:

1. Open editors to the right line based on the filename (turns out the argument is identical to all major editors, which is pretty cool). Anyway, if they don't succeed with the argument, the try to open the editor without.
2. Handles errors for a non-existent $EDITOR and $VISUAL. The user won't get a visual indication (because we sadly don't have an easy way of doing this yet), but the error - along with how to fix it - will appear in the logs.

Going to take a look at your changes now, see what needs to be fixed, then write tests, do some cleanup and we're good I think (unless I'm forgetting something).

[cosminadrianpopescu]

PR opened.

The only minor issues I see are those I've told you about plus another one, also with floating panes. If you open a floating pane, you then edit the scroll buffer, then you embed and then you make float again, you will see both of the panes. This is again, because of self.floating_panes.panes_contain(&id) even when the pane is embedded. And if this is not a bug, then I would say that the behavior is normal, in my oppinion.

What do you think?

[imsnif]

Hum... does this happen with a regular non-embedded editor pane? I don't fully have context on this yet. Just diving in now to your changes.

Can we sum up the issues that we have with the current implementation?

[cosminadrianpopescu]

So, far there are 2 minor issues, in my oppinion:

1. Open floating panes. In the opened floating panel, edit the scroll back. Now your buffer is replaced by the editor. Now embed the panel, now close the editor. You won't see the panel non-embeded. Now, in my oppinion, if self.floating_panes.panes_contain(&id) should return false when the panel id is embedded, then this is a bug. But the bug has to be corrected by correcting the self.floating_panes.panes_contain to return false when the pane id is embedded. Then also this feature should work properly. If self.floating_panes.panes_contain(&id) should return true even if the panel is embedded, then it's normal that after I embeded the editor and closed it I should again open the floating panes to see the edited buffer. Maybe do this automatically in the close_pane function?
2. Same scenario: Open floating panes, in the opened floating panel edit the scroll back. Now embed the panel and now make the editor panel floating again. At this point, you will see the 2 panes (the editor and the edited panel). Here, I am not that sure that even if self.floating_panes.panes_contain(&id) would return false it would work. Or even that it should not work if it returns true.

Unfortunatelly I don't have discord or any other social app (WhatsApp, Teams etc.). I only have signal messenger. If you want, you can send me a private message at cosminadrianpopescu at gmail.com and we can discuss directly via Signal Messenger, if you have it.

[cosminadrianpopescu]

If you don't have signal messenger, I have my own infrastructure at https://cloud.taid.be (my company) where we can do video calls and all the others. Again, give me a message on my private e-mail and I will send you a link, where you can connect and we can talk directly (video and audio or only audio) and we can share screens and everything that you would expect from such a thing.

[imsnif]

Hey @cosminadrianpopescu - I pushed changes into the branch that should fix everything.

I hope you don't mind, but I ended up using the suppressed_panes strategy. I just know this part of the code base very well and know what pitfalls to avoid. There would just be too many things to fix otherwise (as you started seeing with the bugs you mentioned).

This fixed all the bugs you mentioned - including the one with new info coming to the suppressed terminal (eg. with tail). But please check me to make sure I'm not mistaken (when you have the time).

I'm taking a lunch break and afterwards I'm going to write tests and maybe look into placing an editor command override in the config as you suggested (if it won't take too long).

I can wait until tomorrow to merge this if you don't have time to test today.

[cosminadrianpopescu]

I hope you don't mind, but I ended up using the suppressed_panes strategy.

No, of course. Whatever works best and faster.

But please check me to make sure I'm not mistaken (when you have the time).

I will check out a little bit later. Right now I'm not at a computer.

[cosminadrianpopescu]

All works perfect now (floating and tiled panes). I did not find any new issues. I will keep testing and starting Tuesday, I will use it also in a prod like environment (I will build the new version at work) and let you know if I found more issues (I hope not).

I was thinking of one last nice touch. To name the panel rather than "Panel #", maybe we can give it a nice name, like "Edit Panel #" which would then be the number of the panel edited? Or if the panel edited has a given name, then it would be "Edit Panel Custom Name".

How difficult would that be? I might be able to give it one hour this evening later. Do you think is fesable? Or better to leave it for later?

[imsnif]

Nice! I plan on releasing either tomorrow or on Tuesday, so you can already use the release version and if there's a bug we can always release a patch.

I like the title idea. I'd call it "Editing Pane # Scrollback"
I think it shouldn't be too difficult if we pass the title explicitly here: https://github.com/cosminadrianpopescu/zellij/blob/edit-scrollback/zellij-server/src/tab/mod.rs#L714
The problem is that this will always override whatever title the editor sets... Not too bad though - we can work around it later and I think this is a cool default.

[imsnif]

You might need to update some snapshots since I already added tests, but shouldn't be too difficult with cargo insta review.

[cosminadrianpopescu]

I will give it a try and let you know if I can come up with anything. If not, I suppose I can add it at a later stage after release, right? Since today I'm not that free.

[imsnif]

For sure, no stress! As you said, it's a pretty amazing feature as is. I'm very much looking forward to using this in my day-to-day.

[imsnif]

Alright, all is done in the PR.
I added:

1. Tests (several integration to test edge cases and one e2e to test everything together)
2. Some minor fixes here and there
3. A config attribute: scrollback_editor to allow overriding the environment variable
4. A section in zellij setup --check that let's the user know which editor command we're going to use

I also merged from main. So @cosminadrianpopescu - if you want to add the window title sometime tonight, please do go ahead. Otherwise I'll merge this to main tomorrow and release either tomorrow or on Tuesday.

[cosminadrianpopescu]

No, unfortunately I didn't had any time this evening. I'll add that with other release.

[nlvw]

If the scrollback_editor option or $EDITOR env variable has any flags then it fails. This is the case wether there is an empty space at the end or not in the config file.

$ echo $EDITOR
vim -u ~/.config/vim/vimrc.vim

scrollback_editor: '/usr/bin/less --use-color '

[imsnif]

@nlvw I think we were really counting on this just being a path to a command. I'm not sure this is the "intended" behaviour or not, tbh :) Does it work if you wrap it all in bash script to get the flags?

[cosminadrianpopescu]

Does it work if you wrap it all in bash script to get the flags?

I've had the same issue as @nlvw and I can confirm that it works if you wrap it in a script and point the $EDITOR variable to that script.

[imsnif]

Question is... is this a bug or the intended behaviour? I wonder if eg. git picks up the $EDITOR flags when editing merge commits and rebases...

[cosminadrianpopescu]

I found a problem with the current implementation of the feature:

If you have several split panes, then you full screen one, then you replace the editor scrollback and then you close the editor, the pane will get the geometry of the buffer at exit point, which due to full screen was the max geometry. So, when you take the pane out of the full screen, then it will remain maximized. I will look into it.

[imsnif]

I'm closing this issue for now though because the feature itself was already implemented. @cosminadrianpopescu - please keep me updated on the issue. We can always open a new one.

[imsnif]

Implemented in 0.30.0