feat: add initial session name to layout template

[jaeheonji]

This PR resolve #611

proposal

---
session:
  - name: "SESSION_NAME"
template:
  ...
tabs:
  ...

Instead of using session_name, a session structure is used. (for future scalability)

expected results

1. When Zellij is launched (with layouts), the session name is initialized to the name in the layouts.
2. When the -s flag and layouts are used together (like zeliij -s SOMTHING --layout-path SOME.yaml), the flag of session name takes a priority.

[jaeheonji]

I tried to implement the functionality as simple as possible. If this is ok, I'll add an e2e test.

[a-kenji]

Thank you for taking the time submitting this pr!
This is already pretty good.

For now when we start a session with a name, then detach, and then start it again.
We get an error message.

 Use attach command to connect to it or specify a different name.

I would personally prefer if either we could have an option to reattach or not
while already starting with the layout, or at least have the reattach the default option.
What do you think?

[jaeheonji]

@a-kenji thank you for your review :)

That part I didn't even think of! I totally agree your opinion.
so, I'll proceed as follows:

---
session:
  name: 'SESSION_NAME'
  attach: true # default `true`. If session exists, re-attach.

[a-kenji]

@jaeheonji
This would look good to me!

[jaeheonji]

@a-kenji

While developing for attach option, I suddenly had the following thoughts. I'm wondering about your opinion on this.

The layout system is responsible for setting the default UI when Zellij is startup. and The role of attach is to attach to an existing session.

Here, my sudden thought is that it makes more sense(natural) to use the attach command than to put the attach option in the layout.

Because, accessing an existing session through the attach option in the layout file seems to fade the meaning of the rest of the settings in the layout file (like tab, plugins etc..).

Sorry for the sudden change of opinion, but I'd like to ask your opinion on this 😄

[a-kenji]

Hey! While that is certainly true for now, the goal is merging the config file with the layout file in order to make it possible specifying any option on loading the layout. That of course can also just be the layout.

I agree it would make the existing configuration of plugins pointless for that session, but because it already is running with that name I think it is safe to assume most of the layout, if not all would already be running.

[jaeheonji]

Oh.. Thank you for solving my question. The purpose seems to have become clearer :)

[a-kenji]

@jaeheonji
Thank you for the work!
There is still one little inconsistency:
When creating a new session with the -s flag we now attach to an existing
session instead of getting the error.

Apart from that I believe everything does work as expected!

[jaeheonji]

Thanks for review! @a-kenji
I have a schedule today, so I'll update the part you informed me by tomorrow :)

[jaeheonji]

@a-kenji I updated attach options to work only when there is a layout template argument.

And as the main function has more functions, the code seems to be massive. It seems that refactoring is necessary in the future.

EDIT: releated to #824

[a-kenji]

@jaeheonji
This is still a little off from the desired behavior, but very close.
I think rather than check for the layout template argument,
you rather want to check where the layout name is coming from,
since you should be able to create a session, with a name,
and a layout path argument.

Thanks for the great work!
And I agree the main function is in need for some maintenance soon.

[jaeheonji]

@a-kenji Ok! I didn't understand.
Then, let's take a look at the overall flow for confirmation once again.

• -s options always have priority, so If a name is given through the -s option, a session is created with that name. Also, it ignores the attach condition of the layout. (therefore, if the name already exists, an error is displayed.)

• If there is no -s option, the name of the layout is checked.

  • If the name of the layout does not exist or the layout itself does not exist, a session is created with a random name.
  • If the layout has a name:
    • If a session with the same name already exists, it connects to the session or displays an error according to the attach option in layout. (the default of attach is true)
    • Otherwise, create a session with the name of the layout.

Is this the behavior we expect?

[a-kenji]

@jaeheonji
Yes, that is exactly the behavior I think we would expect!
Sorry for not being clearer before!

[jaeheonji]

Good! We all don't have to be sorry :)

If the above is implemented, the length of the code will be longer and there will be many overlapping parts. (probably there will be many flows)

So, I'll try the implementation once with refactoring.

[a-kenji]

@jaeheonji
Just a heads up, in #829 there is a refactor of the main function already.
Do you wish to base upon that refactor?
It seems to make things much clearer.

[jaeheonji]

@a-kenji
That would be good. (I have now confirmed that #829 is going on.)

The work on #829 is exactly what I was looking for. After the refactoring is done, it would be nice to implement it in that code base.

[ken-matsui]

Thank you all so much for considering using my refactor. It still has a bit confusing parts, so I really would welcome your suggestion!

[jaeheonji]

@a-kenji

I added features (#789 (comment)) from the latest sources (refactored). please could you do a review?

[a-kenji]

LGMT!

[a-kenji]

@jaeheonji,
thank you for keeping at it!
This looks great now I think.