[WIP] Left and right dockarea #3123

vesim987 · 2018-01-24T23:41:34Z

This PR add support for vertical dock client.

TODO:

handle_strut_partial_change seem's to doesn't work
do something with dock overlapping
refactor render_output
write tests

fixes: #1129

vesim987 · 2018-01-24T23:42:35Z

Oh, i broke commit history. How i can fix it?

okraits · 2018-01-25T06:49:51Z

I guess you need to rebase your branch against the next branch.

Airblader · 2018-01-25T07:38:26Z

Thanks for taking a shot at this! I really think this would be a cool feature to have in i3. As @okraits mentioned you'll need to rebase.

Once that is sorted out, two very high-level comments (I haven't reviewed the PR in detail yet):

It'd be good if you could write, in text form, some explanation as to how vertical dock clients fit into the container structure of i3 now. For one it'll help us understand, but I also think we'll need to document this a bit in, say, the hacking howto, as this is a change to the fundamental container structure.
Please don't rename things on the IPC (I just saw a "dockarea" → "hdockarea"). This breaks compatibility which we'd like to avoid when possible. Having "dockarea" and "vdockarea" is not the ideal situation, but it's better than breaking IPC clients.

Let us know via comment when you rebased the PR!

vesim987 · 2018-01-25T14:20:59Z

@Airblader I've fixed this branch. There was ~170 commits difference... idk how I did this. I've just created new branch, and applied my latest commit, and renamed it.

Airblader · 2018-01-26T08:13:59Z

add left and right position to i3bar

Please don't do this in this PR. It's a completely separate topic and one that has to be decided separately as well. I currently don't think we want a vertical mode for i3bar because we have no good answer to what the statusline should look like.

Airblader

I did a quick initial pass over the changes, but I haven't yet looked at it too closely. I think there's some stuff to work on still and would also appreciate the textual description I mentioned previously.

Airblader · 2018-01-26T08:14:39Z

src/con.c

@@ -535,7 +536,10 @@ bool con_is_docked(Con *con) {
    if (con->parent == NULL)
        return false;

-    if (con->parent->type == CT_DOCKAREA)
+    if (con->parent->type == CT_HDOCKAREA )


Here and in general, please run the clang formatter to fix the code formatting.

Airblader · 2018-01-26T08:15:00Z

src/con.c

+    if (con->parent->type == CT_HDOCKAREA )
+        return true;
+
+    if (con->parent->type == CT_VDOCKAREA )


Why not combine these two conditions?

Airblader · 2018-01-26T08:17:12Z

src/con.c

@@ -2124,7 +2132,7 @@ void con_update_parents_urgency(Con *con) {
        return;

    bool new_urgency_value = con->urgent;
-    while (parent && parent->type != CT_WORKSPACE && parent->type != CT_DOCKAREA) {
+    while (parent && parent->type != CT_WORKSPACE && parent->type != CT_HDOCKAREA && parent->type != CT_VDOCKAREA ) {


In general I think it'd be good to use something like con_is_docked for all of these conditions since they get pretty lengthy.

Airblader · 2018-01-26T08:23:03Z

src/handlers.c

-            DLOG("geom->y = %d >= rect.height / 2 = %d, it is a bottom dock client\n",
-                 con->geometry.y, (search_at->rect.height / 2));
-            con->window->dock = W_DOCK_BOTTOM;
+    if(abs(con->window->reserved.bottom - con->window->reserved.top) > 


This entire block has become quite unreadable. I think we'd really gain a lot of clarity from refactoring this a little bit (if you want).

Airblader · 2018-01-26T08:23:30Z

src/ipc.c

-        case CT_DOCKAREA:
-            ystr("dockarea");
+        case CT_HDOCKAREA:
+            ystr("hdockarea");


Please revert this to dumping "dockarea" to maintain compatibility on the IPC.

Airblader · 2018-01-26T08:24:02Z

src/load_layout.c

@@ -340,8 +340,10 @@ static int json_string(void *ctx, const unsigned char *val, size_t len) {
                json_node->type = CT_FLOATING_CON;
            else if (strcasecmp(buf, "workspace") == 0)
                json_node->type = CT_WORKSPACE;
-            else if (strcasecmp(buf, "dockarea") == 0)
-                json_node->type = CT_DOCKAREA;
+            else if (strcasecmp(buf, "hdockarea") == 0)


And this would have to be adjusted back as well

Airblader · 2018-01-26T08:24:59Z

src/randr.c

@@ -316,6 +316,7 @@ void output_init_con(Output *output) {
    }
    con->rect = output->rect;
    output->con = con;
+


Please remove this extra newline

Airblader · 2018-01-26T08:25:25Z

src/randr.c

-    DLOG("attaching\n");
-    con_attach(topdock, con, false);
-
+    {


These blocks absolutely do need to be refactored; using scoped blocks with this much duplication is not good. :-)

Airblader · 2018-01-26T09:33:02Z

src/render.c

@@ -338,41 +343,100 @@ static void render_output(Con *con) {

    /* First pass: determine the height of all CT_DOCKAREAs (the sum of their
     * children) and figure out how many pixels we have left for the rest */
+    Con *left_dock = 0;


Please use NULL over 0

Airblader · 2018-01-26T09:33:56Z

src/render.c

+    }
+
+
+    if(bottom_dock) {


Again, these blocks are a lot of repetition that should be refactored.

vesim987 · 2018-01-26T14:32:02Z

@Airblader do you have idea how to rewrite this part: https://github.com/vesim987/i3/blob/vertical-dockarea/src/handlers.c#L1376-L1415

op8867555 · 2018-02-10T09:59:07Z

Hi there, any progress on this? I tried this PR and found some problems:

Incorrect bar position when using multiple monitors
Incorrect bar position when running multiple bars
is_docked is not restore when loading json layout which would crash i3 when reloading

Here is some changes to fixes problems above:
vesim987/i3@vertical-dockarea...op8867555:6f4ba84f540a4ac74bef8fd09a2f495911e913ed

I'm new to i3, please take a look and point me out if I didn't do it right. :)

Airblader · 2018-06-22T11:45:00Z

@vesim987 Are you continuing your work on this?

Added initial support for vertial dock clients

42b0fc9

vesim987 force-pushed the vertical-dockarea branch from b5aa7f7 to 42b0fc9 Compare January 25, 2018 14:12

Airblader requested changes Jan 26, 2018

View reviewed changes

Vesim987 added 2 commits January 26, 2018 15:19

Refactored output_init_con and added is_docked to Con structure.

97da015

Renamed hdockarea ipc node name

e473786

Airblader added the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Mar 24, 2018

Airblader added the stale Issue or PR has become stale and will be closed soon label Jun 22, 2018

Airblader closed this Jul 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Left and right dockarea #3123

[WIP] Left and right dockarea #3123

vesim987 commented Jan 24, 2018 •

edited

Loading

vesim987 commented Jan 24, 2018 •

edited

Loading

okraits commented Jan 25, 2018

Airblader commented Jan 25, 2018

vesim987 commented Jan 25, 2018

Airblader commented Jan 26, 2018

Airblader left a comment

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

Airblader Jan 26, 2018

vesim987 commented Jan 26, 2018

op8867555 commented Feb 10, 2018 •

edited

Loading

Airblader commented Jun 22, 2018

[WIP] Left and right dockarea #3123

[WIP] Left and right dockarea #3123

Conversation

vesim987 commented Jan 24, 2018 • edited Loading

vesim987 commented Jan 24, 2018 • edited Loading

okraits commented Jan 25, 2018

Airblader commented Jan 25, 2018

vesim987 commented Jan 25, 2018

Airblader commented Jan 26, 2018

Airblader left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vesim987 commented Jan 26, 2018

op8867555 commented Feb 10, 2018 • edited Loading

Airblader commented Jun 22, 2018

vesim987 commented Jan 24, 2018 •

edited

Loading

vesim987 commented Jan 24, 2018 •

edited

Loading

op8867555 commented Feb 10, 2018 •

edited

Loading