Deduplicate code in zellij-server screen

[har7an]

Hi,

this is an attempt to deduplicate the code for Screen in zellij-server. It's made up of three distinct "improvements" (if I didn't mess up the partial adds, mind):

1. 4b5c63e Replaces the following code blocks:

               screen
                   .bus
                   .senders
                   .send_to_server(ServerInstruction::UnblockInputThread)
                   .unwrap();

   with a new function:

               screen.unblock_input();

2. 9c2e0ef deduplicates the following constructs:

                       if let Some(active_tab) = screen.get_active_tab_mut(client_id) {
                           active_tab.new_pane(pid, Some(client_id));
                       } else {
                           log::error!("Active tab not found for client id: {:?}", client_id);
                       }

   with a new macro:

                       active_tab!(screen, client_id, |tab: &mut Tab| tab
                           .new_pane(pid, Some(client_id)));

3. Is a small change and consists of moving screen.render() to the bottom of the inner loop after the match, so it is always executed by those match arms that don't explicitly "opt-out" (by calling continue).

I hope that this aids the codes readability. Happy to take any feedback you can give me!

[har7an]

Also I read in one of the previously existent comments that screen.update_tabs() calls screen.render() eventually, which is why I added a continue after every update_tabs(). Is this really the case or has this changed in the meantime?

[har7an]

@jaeheonji I fixed the merge conflicts, could you restart the pipelines, please?

Also I hope I didn't miss any of the newly added commands...

[har7an]

Hey @a-kenji could you maybe have a look at these changes? @imsnif seems to be busy and due to the many modifications to screen.rs I fear that soon I will miss a change when rebasing...

[imsnif]

Hey @har7an ! The third refactor (with the render) while very important both for readability and even performance, is not necessarily a trivial one. Maybe it is, but there's quite some hidden and implied behaviour around when and how many times we render.

I'd definitely like to address that part separately. Since I'm going on vacation for a couple of weeks now - how about we separate that particular piece out and go ahead with the first two refactors if @jaeheonji think they're good? (I haven't looked through the code myself)

Then we can potentially tackle the rendering as a separate issue (and I think we can even include some more performance and readbility optimizations there).

[har7an]

The third refactor (with the render) while very important both for readability and even performance, is not necessarily a trivial one. Maybe it is, but there's quite some hidden and implied behaviour around when and how many times we render.

Humm, I was suspecting there's more to it. :)
I mean I wrote it such that it doesn't change the status quo except that I removed all calls to render when there is an update_tabs before it. But I will leave that decision to you, since you're the one responsible for the code and won't be here to extinguish any fires it causes. ;)

Since I'm going on vacation for a couple of weeks now - how about we separate that particular piece out and go ahead with the first two refactors

That's fine by me. I understand that you will want to have a look at the rendering mechanism first before you make any decisions regarding that? In this case I won't be looking at it any further until you return.

In the meantime if you know of some other files that contain amounts of repetition similar to screen.rs I could offer to have a stab at these as well.

[har7an]

@imsnif I dropped the third commit, I have it stashed locally in case we want to come back to it some day.

Enjoy your vacation!

[jaeheonji]

@imsnif Thank you for your commenting! Good. At this point, I think it would be better to proceed with only the previous two refactorings. (Hope you have a nice vacation 😉)

@har7an Thank you for your efforts :) I will merge as soon as CI is completed.