Re: [PATCH] New command: use_last

[Thorsten Wißmann <re06huxa _at_ cip _dot_ cs _dot_ fau _dot_ de>]

Hi Florian,

On Wed, Oct 31, 2012 at 12:01:20PM +0100, Florian Bruhin wrote:
> Aaaaand another patch, since some people in IRC wanted an easy way to
> do this.
> 
> This patch implements an use_last command which jumps to the tag which
> was focused before switching to the tag which is focused now.

Thanks for your effort but I won't merge this because there is a serious
bug, which is triggered by:

chain , add foo
      , add bar
      , use foo
      , use bar
      , merge_tag foo
      , use_last

> diff --git a/src/main.c b/src/main.c
> @@ -592,7 +593,7 @@ void monitor_set_tag(HSMonitor* monitor, HSTag* tag) {
>          }
>          return;
>      }
> -    HSTag* old_tag = monitor->tag;
> +    monitor->last_tag = monitor->tag;
>      // 1. show new tag

If you only remember an HSTag*-Pointer, then you won't notice it if it
gets invalid.

> +int monitor_set_last_tag_command(int argc, char** argv) {
> +    (void) argc;
> +    (void) argv;
> +    HSMonitor* monitor = get_current_monitor();
> +    HSTag* tag = monitor->last_tag;
> +    if (monitor && tag) {
> +        monitor_set_tag(monitor, tag);

So it will crash here.

There are different approaches to fix this:

  - Remember the tag name and then switch to it. Its downside is: This
    will cause unexpected behaviour if the tag "foo" in the above
    example is recreated before use_last is called.

    This is exactly what scripts/lasttag.sh does. (The command to
    trigger it is emit_hook goto_last_tag).

  - When removing a tag "t", then do the following for each monitor m:
    If the m->last_tag points to tag "t", then set m->last_tag to NULL.
    Its downside is: it somehow feels hacky and not very clean.

I'd prefer the second option. But anyways: Do you think we really need
the use_last command if there is this scripts/lasttag.sh? If you insist
on the command use_last, then please add also a comment at the beginning
of scripts/lasttag.sh that this script is obsolete but still is kept as
an example script for using custom hooks.

Thorsten