feature request: enhanced error message for select-layout

[Thomas Sattler]

Hi Nicholas,

I wanted to reboot a machine where I had created a layout with
~10 panes inside a single window. So i dumped the layout with

$ tmux display-message -p "#{window_layout}"

to a file and tried to restore it after the reboot.


It (read: I) failed a few times, as I did not split my window
into the correct number of panes before I tried to restore
the layout with

$ tmux select-layout $layout


Would it be possible to extend the error message

can't set layout: $layout

and mention, why that failed (not enough or to many panes)?

Or make select-layout create panes if necessary? But I'd be
more than happy to be told to increase/decrease the number
of panes myself.


I also tried to read the layout string but could not find
out how much panes were needed. The mentioned layout was:

ac67,319x81,0,0[319x41,0,0{206x41,0,0,0,72x41,207,0[72x20,
207,0,1,72x20,207,21,98],39x41,280,0,3},319x39,0,42{122x39,
0,42,4,124x39,123,42,9,35x39,248,42,8,35x39,284,42,7}]

Thomas

[Nicholas Marriott]

yes better error would be good, the layout code would need to return an error cause string up to the command - that should be pretty simple

I'm not sure how easy it would be to make it create any extra panes

--
You received this message because you are subscribed to the Google Groups "tmux-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to tmux-users+...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/tmux-users/f1070f82-0818-a9e8-6544-9f66b2849093%40med.uni-frankfurt.de.

[Nicholas Marriott]

Try this please which changes the error messages:

Index: cmd-select-layout.c
===================================================================
RCS file: /cvs/src/usr.bin/tmux/cmd-select-layout.c,v
retrieving revision 1.40
diff -u -p -r1.40 cmd-select-layout.c
--- cmd-select-layout.c 21 Aug 2021 10:22:39 -0000 1.40
+++ cmd-select-layout.c 5 Apr 2022 10:58:00 -0000
@@ -77,7 +77,7 @@ cmd_select_layout_exec(struct cmd *self,
struct window *w = wl->window;
struct window_pane *wp = target->wp;
const char *layoutname;
- char *oldlayout;
+ char *oldlayout, *cause;
int next, previous, layout;

server_unzoom_window(w);
@@ -124,8 +124,9 @@ cmd_select_layout_exec(struct cmd *self,
}

if (layoutname != NULL) {
- if (layout_parse(w, layoutname) == -1) {
- cmdq_error(item, "can't set layout: %s", layoutname);
+ if (layout_parse(w, layoutname, &cause) == -1) {
+ cmdq_error(item, "%s: %s", cause, layoutname);
+ free(cause);
goto error;
}
goto changed;
Index: layout-custom.c
===================================================================
RCS file: /cvs/src/usr.bin/tmux/layout-custom.c,v
retrieving revision 1.20
diff -u -p -r1.20 layout-custom.c
--- layout-custom.c 11 Mar 2021 06:31:05 -0000 1.20
+++ layout-custom.c 5 Apr 2022 10:58:01 -0000
@@ -154,7 +154,7 @@ layout_check(struct layout_cell *lc)

/* Parse a layout string and arrange window as layout. */
int
-layout_parse(struct window *w, const char *layout)
+layout_parse(struct window *w, const char *layout, char **cause)
{
struct layout_cell *lc, *lcchild;
struct window_pane *wp;
@@ -165,22 +165,30 @@ layout_parse(struct window *w, const cha
if (sscanf(layout, "%hx,", &csum) != 1)
return (-1);
layout += 5;
- if (csum != layout_checksum(layout))
+ if (csum != layout_checksum(layout)) {
+ *cause = xstrdup("invalid layout");
return (-1);
+ }

/* Build the layout. */
lc = layout_construct(NULL, &layout);
- if (lc == NULL)
+ if (lc == NULL) {
+ *cause = xstrdup("invalid layout");
return (-1);
- if (*layout != '\0')
+ }
+ if (*layout != '\0') {
+ *cause = xstrdup("invalid layout");
goto fail;
+ }

/* Check this window will fit into the layout. */
for (;;) {
npanes = window_count_panes(w);
ncells = layout_count_cells(lc);
- if (npanes > ncells)
+ if (npanes > ncells) {
+ xasprintf(cause, "too many panes (need %u)", ncells);
goto fail;
+ }
if (npanes == ncells)
break;

@@ -217,8 +225,10 @@ layout_parse(struct window *w, const cha
}

/* Check the new layout. */
- if (!layout_check(lc))
+ if (!layout_check(lc)) {
+ *cause = xstrdup("size mismatch after applying layout");
return (-1);
+ }

/* Resize to the layout size. */
window_resize(w, lc->sx, lc->sy, -1, -1);
Index: tmux.h
===================================================================
RCS file: /cvs/src/usr.bin/tmux/tmux.h,v
retrieving revision 1.1166
diff -u -p -r1.1166 tmux.h
--- tmux.h 24 Mar 2022 09:05:57 -0000 1.1166
+++ tmux.h 5 Apr 2022 10:58:01 -0000
@@ -3017,7 +3017,7 @@ void layout_spread_out(struct window_p

/* layout-custom.c */
char *layout_dump(struct layout_cell *);
-int layout_parse(struct window *, const char *);
+int layout_parse(struct window *, const char *, char **);

/* layout-set.c */
int layout_set_lookup(const char *);

[Thomas Sattler]

Hi Nicholas,

thanks, that patch already helps a lot.

I think it might be worth to include npanes in the message:

xasprintf(cause, "need %u panes (have %u)", ncells, npanes);

And, last thought, as we then have a meaningful error message,
tmux might also refuse applying the layout when there are not
enough panes for the requested layout.

As far as I understand the code, currently the layout's cells
are kind of "silently reduced" to the number of existing panes.

I tried to implement the idea above and attached a patch that
should be applied after your patch. Please have a close look,
as I might have misunderstood the code I changed.

Thomas

[Nicholas Marriott]

Well, we could do this, but that means people can't apply layouts without exactly the right number of panes, is that really better?

[Thomas Sattler]

I can obviously only speak for myself. But when I save/restore
a layout, I'd expect to get exactly what I had before. Or an
error message, telling me why I cannot get it.

Even when I would want to re-use a saved layout for a "similar
but not identical" use case, I don't think that generally the
bottom right panes would always be the ones I don't need any-
more: I think I would restore the exact layout and than use
that as a starting point, and modify it for my new needs.

But as I wrote above: I can only speak for myself.



Am 05.04.22 um 19:47 schrieb Nicholas Marriott:

[Nicholas Marriott]

I can understand that, but it seems sensible to me to allow layouts with fewer panes to be applied somehow, not to mention that it has behaved like this for 12 years. If tmux could create missing panes then this would not come up; I think it would be better to leave it as it is until then.

[Thomas Sattler]

I'm already quite happy when the restore command tells me that I
have too many panes for the requested layout. Even better if it
shows both, current number of panes and number of cells in the
layout. Or maybe just the difference (to have only one number
in the message and thus make it a bit shorter).

Another final thought: As there now is a way to inform the user,
how about giving the user a small hint that the requested layout
did not exactly match? That was what surprised me most: That a
not exactly matching layout is silently modified.



Am 06.04.22 um 10:57 schrieb Nicholas Marriott:

> I can understand that, but it seems sensible to me to allow layouts with
> fewer panes to be applied somehow, not to mention that it has behaved
> like this for 12 years. If tmux could create missing panes then this
> would not come up; I think it would be better to leave it as it is until
> then.
>
>
> On Tue, 5 Apr 2022 at 23:16, Thomas Sattler
> <sat...@med.uni-frankfurt.de <mailto:sat...@med.uni-frankfurt.de>> wrote:
>
> I can obviously only speak for myself. But when I save/restore
> a layout, I'd expect to get exactly what I had before. Or an
> error message, telling me why I cannot get it.
>
> Even when I would want to re-use a saved layout for a "similar
> but not identical" use case, I don't think that generally the
> bottom right panes would always be the ones I don't need any-
> more: I think I would restore the exact layout and than use
> that as a starting point, and modify it for my new needs.
>
> But as I wrote above: I can only speak for myself.
>
>
>
> Am 05.04.22 um 19:47 schrieb Nicholas Marriott:
> > Well, we could do this, but that means people can't apply layouts
> > without exactly the right number of panes, is that really better?
> >
> >
> >
> > On Tue, 5 Apr 2022 at 17:34, Thomas Sattler
> > <sat...@med.uni-frankfurt.de
> <mailto:sat...@med.uni-frankfurt.de>

> <mailto:sat...@med.uni-frankfurt.de

[Thomas Sattler]

> Another final thought: As there now is a way to inform the user,
> how about giving the user a small hint that the requested layout
> did not exactly match? That was what surprised me most: That a
> not exactly matching layout is silently modified.

Just to be clear: Restore as before, but tell the user that some
cells in the given layout were ignored as not enough panes exist.

[Nicholas Marriott]

There is no mechanism for information messages, it can either fail or succeed.

The only option is to fail and provide a flag to force (ie to tell it to remove cells if it wants) but I think that is overkill for this.

[Thomas Sattler]

OK, my mistake, the message is only for when a layout cannot be applied?
Then let's forget about that part. Keeping a 10+ years behaviour does of
cause beat my sense of mismatching layouts. Especially as it seems that
nobody else complained about it within that time.


How do you think about changing the error message to something like

too many(3) panes to apply layout

or

too many panes (10 vs 7) to apply layout

Trying to say there are three more panes than cells in the layout.
Maybe there are ways to express that shorter or more beautiful (I
am not a native English speaker).



Am 06.04.22 um 15:17 schrieb Nicholas Marriott:

[Nicholas Marriott]

I applied this but changed it to:

+                       xasprintf(cause, "have %u panes but need %u", npanes,
+                           ncells);