bakkeby / patches

Collection of patches for dwm, st and dmenu
284 stars 30 forks source link

[Bug] Scratchpad window always creates new tags when executed #64

Closed adetabrani closed 1 year ago

adetabrani commented 1 year ago

i'm trying to combine tagicons patch with scratchpads patch, i'm trying to make them compatible with each other, but i get a bug that makes the scratchpad window always create a new tag when executing, is there a way to prevent the creation of a new tag when executing scratchpad?

here is my repo to try it

simplescreenrecorder.webm

bakkeby commented 1 year ago

The way the scratchpads patch works is precisely by adding a new tag and enabling that tag in addition to your currently selected tag(s) to show the scratchpad.

This means that in every single place where you refer to e.g. c->tags you need to filter out these extra tags by applying SPTAGMASK (e.g. c->tags & SPTAGMASK).

In your specific case though this happens because in the drawbar function where the tags are drawn we loop through NUMTAGS which includes the scratchpads.

#define NUMTAGS                         (9 + LENGTH(scratchpads))

There are ways to work around this issue, but I would just want to say that there is a fair deal of hassle having to take into account these "hidden" tags for this scratchpads patch and I would much rather recommend the namedscratchpads patch as it is much easier to work with. I have a more bloated version of that patch named renamedscratchpads as well if that would be of interest.

adetabrani commented 1 year ago

thank you for the recommendation, how do I bind with other utilities (eg: ncmpcpp) on renamedscratchpads?

bakkeby commented 1 year ago

As with all scratchpad patches we need to have a client rule that matches the window and marks that client as a scratchpad.

ncmpcpp specifically is a terminal program, so this will need to be run in a terminal.

Many terminal applications, but not all, allow for the class and/or instance property of the window to be set via command line options. Using st for this example as it is easy for demonstration purposes.

Let's say that I have a script or alias that runs the following command to start ncmpcpp in a dedicated terminal.

st -c "thefancyncmpcppterminal" -e ncmpcpp

The -e flag executes the command that follows, i.e. the ncmpcpp program, while the -c flag sets the class of the window.

In the client rule we can now filter on the class of "thefancyncmpcppterminal" and mark that as a scratchpad.

Rule:

    /* class                         instance    title       tags mask     isfloating   monitor    scratch key */
    { "thefancyncmpcppterminal",     NULL,       NULL,       0,            1,           -1,        'n'  },

Command:

static const char *ncmpcppcmd[] = {"n", "st", "-c", "thefancyncmpcppterminal", "-e", "ncmpcpp", NULL};

Keybindings:

    { MODKEY,                       XK_n,      togglescratch,  {.v = ncmpcppcmd } },
    { MODKEY|ShiftMask,             XK_n,      removescratch,  {.v = ncmpcppcmd } },
    { MODKEY|ControlMask,           XK_n,      setscratch,     {.v = ncmpcppcmd } },

Personally I have found it easier to navigate the keybindings, command and rule if I use the same scratch key ("n") as the dedicated keybinding I am using (XK_n in this case).


The above example is intentionally more elaborate than it needs to be. This because it describes a more generic approach that can be used in more situations.

Quite strictly in this particular case you can get away with a command of:

static const char *ncmpcppcmd[] = {"n", "st", "-e", "ncmpcpp", NULL};

and a rule of:

    /* class    instance    title       tags mask     isfloating   monitor    scratch key */
    { NULL,     NULL,       "ncmpcpp",  0,            1,           -1,        'n'  },

because the window title will be set to "ncmpcpp" when the terminal starts. That won't always be the case though.

adetabrani commented 1 year ago

Wow big thanks, I have a last question maybe this is not related, I tried bind st with lf (terminal file manager) as a scratchpad but it doesn't seem to work well, there just pops up a window like flicker. I don't know if this is a bug in LF or Dwm or even only occurs on my OS (fedora 38), but this bug does not occur in similar utilities such as ranger, have you ever experienced this bug?

For more details, see the following video simplescreenrecorder.webm

bakkeby commented 1 year ago

In the screenshot I only see the window title of "lfcd", so presumably you are running something like: https://github.com/gokcehan/lf/blob/master/etc/lfcd.cmd

What is the command that you have set up?

In general though if you run a command that completes then that will terminate the terminal emulator as well.

E.g. this will run ls and immediately exit because the command finishes.

st -e ls

The same happens if you open a terminal emulator and it starts, say, bash and you run the exit command or press Ctrl+d then the shell process exits and thus the terminal emulator closes as well.

If you want the terminal to stay after the command is run then you will want to spawn a shell, run the command, then exec to another shell again.

st -e "bash" -c "ls; exec bash"
adetabrani commented 1 year ago

I use lfcd for added functionality so that I can exit the current directory when I exit lf I've updated it back to lf for testing, I apologize in advance because my scripting skills are still amateur 😔

This is the rule script that I apply, I seem to have succeeded in bringing up the scratchpad but it doesn't access lf, it just brings up a regular terminal popup

static const char *lfcmd[] = {"l", "st", "-c", "lf; exec bash", "-e", "bash", NULL};

bakkeby commented 1 year ago

OK, so I run your command and it brings up a regular terminal.

$ st -c "lf; exec bash" -e "bash"

If I run xprop on that window I get this.

$ xprop | grep WM_CLASS
WM_CLASS(STRING) = "st", "lf; exec bash"

Do you see what is going on?

If you check the man page for st then we have that -e is the last argument, and anything that follows that are arguments for the command.

st ... [[-e] command [arguments...]]

So in your command you are setting the class to "lf; exec bash" because you pass the -c argument to st. If you pass it to the bash command then it works as expected.

$ st -e "bash" -c "lf; exec bash"
static const char *lfcmd[] = {"l", "st", "-e", "bash", "-c", "lf; exec bash", NULL};
adetabrani commented 1 year ago

now I can access lf, but the toggle function doesn't work, it keeps bringing up a new lf window when toggling, it looks like the rule I applied doesn't work

This is the rule I apply:

static const Rule rules[] = {
    /* xprop(1):
     *  WM_CLASS(STRING) = instance, class
     *  WM_NAME(STRING) = title
     *  _NET_WM_WINDOW_TYPE(ATOM) = wintype
     *  WM_WINDOW_ROLE(STRING) = role
     */
    /* class             role             instance  title                wintype,          tags mask  switchtag   isfloating  alwaysontop isterminal noswallow  floatpos                monitor   scratch key */
    { NULL,              NULL,            NULL,     NULL,                WTYPE "DIALOG",   0,         0,          1,          1,          0,         0,         NULL,                   -1,       0 },
    { NULL,              NULL,            NULL,     NULL,                WTYPE "UTILITY",  0,         0,          1,          1,          0,         0,         NULL,                   -1,       0 },
    { NULL,              NULL,            NULL,     NULL,                WTYPE "TOOLBAR",  0,         0,          1,          1,          0,         0,         NULL,                   -1,       0 },
    { NULL,              NULL,            NULL,     NULL,                WTYPE "SPLASH",   0,         0,          1,          1,          0,         0,         NULL,                   -1,       0 },
    { "St",              NULL,            NULL,     NULL,                NULL,             0,         0,          0,          0,          1,         0,         NULL,                   -1,       0 },
    { "firefox",         NULL,            NULL,     NULL,                NULL,             2,         3,          0,          0,          0,         0,         NULL,                   -1,       0 },
    { "firefox",         "Organizer",     NULL,     NULL,                NULL,             0,         0,          1,          0,          0,         0,         "50% 50% 2000W 1200H",  -1,       0 },
    { NULL,              "pop-up",        NULL,     NULL,                NULL,             0,         0,          1,          1,          0,         0,         NULL,                   -1,       0 },
    { NULL,              NULL,            NULL,     "Event Tester",      NULL,             0,         0,          0,          0,          0,         1,         NULL,                   -1,       0 },
    { NULL,              NULL,            NULL,     "scratchpad",        NULL,             0,         0,          1,          0,          1,         0,         "50% 50% 2000W 1200H",  -1,       's' },
    { "ncmpcppterm",     NULL,            NULL,     NULL,                NULL,             0,         0,          1,          0,          1,         0,         "50% 50% 2000W 1200H",  -1,       'n' },
    { "lf; exec bash",   NULL,            NULL,     NULL,                NULL,             0,         0,          1,          0,          1,         0,         "50% 50% 2000W 1200H",  -1,       'l' },
};

simplescreenrecorder.webm

bakkeby commented 1 year ago

That's a bit funny though, sorry.

{ "lf; exec bash",   NULL,            NULL,     NULL,                NULL,             0,         0,          1,          0,          1,         0,         "50% 50% 2000W 1200H",  -1,       'l' },

^ the class was only like that because you passed the -c argument to st rather than bash.

Without it the class will be the default, e.g. st and instance of St.

You can pass another class to make it easier to identify in your rules. For example:

$ st -c "mylifeisbeautiful" -e "bash" -c "lf; exec bash"
adetabrani commented 1 year ago

I told you I'm very stupid when it comes to scripts 🤣, ok now it's working normally, thank you very much for your help, I always look forward to the cool patches that will come to your repository

adetabrani commented 1 year ago
st -c "mylifeisbeautiful" -e "bash" -c "lf; exec bash"

hey I just noticed my lf preview script doesn't work with this command, how do I make the preview work?

This is the LF script that I implemented for the preview, link1 & link2

bakkeby commented 1 year ago

In general when a process spawn another process then the new process will inherit the environment variables of the parent process.

When you have, say, dwm spawn a new process then that process will inherit the environment that dwm lives in - and dwm again inherits that from whatever runs dwm, be that startx or a display manager.

As such we have that when spawning a scratchpad in this case the result may be different compared to when running the same command in a terminal because the latter will have more things set up, i.e. the starting conditions will be different.

I don't know exactly what is wrong in your case, but I see that the preview script depends on an environment variable FIFO_UEBERZUG

.config/lf/preview:4:  printf '{"action": "add", "identifier": "PREVIEW", "x": "%s", "y": "%s", "max_width": "%s", "max_height": "%s", "scaler": "contain", "path": "%s"}\n' "$4" "$5" "$(($2-1))" "$(($3-1))" "$1" > "$FIFO_UEBERZUG"

which is exported in bin/lfub

bin/lfub:18:    export FIFO_UEBERZUG="$HOME/.cache/lf/ueberzug-$$"

which in turn is run instead of lf because of this alias:

.zshrc:104:alias lf="lfub"

My guess is that this alias does not exist (we are running bash of course, not zsh) and plain lf is executed instead.

I think that you would need to try a few things, for example executing zsh instead of bash or running /full/path/to/bin/lfub instead of lf. If your bin directory is in your PATH environment variable (for the dwm process mind you) then you may get away with just running lfub.

adetabrani commented 1 year ago

thanks for the feedback, I think the script above will execute the alias too, in this case "lf=lfub" in my .zshrc, have replaced it with lfub and it works fine

adetabrani commented 8 months ago

Hi bak, can you tell me how to create a scratchpad rule for open terminal in CWD? I'm confused about how to apply it to this part, I want to immediately execute my CWD script without having to set ST first, so the output expected by the CWD terminal will immediately appear when I call the scratchpad

/*First arg only serves to match against key in rules*/
static const char *scratchpadcmd[] = {"s", "st", "-c", "stterm", NULL};
static const char *ncmpcppcmd[] = {"n", "st", "-c", "ncmpcppterm", "-e", "ncmpcpp", NULL};
static const char *lfcmd[] = {"l", "st", "-c", "lfterm", "-e", "zsh", "-c", "lfrun; exec zsh", NULL};
static const char *translatecmd[] = {"t", "st", "-c", "translateterm", "-e", "zsh", "-c", "gtt; exec zsh", NULL};

this is my script for cwd

#!/bin/bash

ID=$(xdpyinfo | grep focus | cut -f4 -d " ")
PID=$(($(xprop -id $ID | grep -m 1 PID | cut -d " " -f 3) + 2))
if [ -e "/proc/$PID/cwd" ]
then
    st -d $(readlink /proc/$PID/cwd) &
else
    st
fi

also it is possible to create scratchpad rule for other application (eg: inkscape)? because on the wiki, I only found how to set scratchpad rules for the terminal only

bakkeby commented 8 months ago

The main problem with using /proc/$PID/cwd is that, in my experience, most graphical applications do not actually set this, but yes this will likely work with other terminals.

That is why patches such as spawn cwd rely on the window title for this sort of thing simply because it tends to be more accurate. Usually one would want this behaviour when working with a file editor and these tend to have the current file path in the window title.

also it is possible to create scratchpad rule for other application (eg: inkscape)? because on the wiki, I only found how to set scratchpad rules for the terminal only

Yes you can set up scratchpads for pretty much every window. It helps if you are able to uniquely identify the window by overriding the window title, class or instance - especially if you also use the same program as a normal (non-scratchpad) window.

I wrote up a guide on how to set up scratchpads which covers most of the patch variants out there, perhaps it may be of some help. https://github.com/bakkeby/dwm-flexipatch/discussions/408

adetabrani commented 8 months ago

I wrote up a guide on how to set up scratchpads which covers most of the patch variants out there, perhaps it may be of some help. bakkeby/dwm-flexipatch#408

thanks for the reference

That is why patches such as spawn cwd rely on the window title for this sort of thing simply because it tends to be more accurate. Usually one would want this behaviour when working with a file editor and these tend to have the current file path in the window title.

Yep, I'm in this situation now (I'm just getting into programming where I started with C, where I always have to switch to the project directory to compile and do git diff when opening new terminal, do you have a script reference or other way to make the CWD window set to a scratchpad rule? who can then do toggling. Sorry if my question might be too basic

bakkeby commented 8 months ago

I have been using the spawn cwd patch in my own build for a long time and that works really well for me.

I suppose that you could also do this with an external script like you propose.

Here is an example wrapper script that I named spawn_cwd.sh.

#!/bin/sh
CWD=$(xdotool getactivewindow getwindowname | sed -r -e "s|~|${HOME}|" -e 's|/[^/]+$|/|' -e 's|^[^~/]+||')
if [ -d "$CWD" ] && [ "$CWD" != "/" ]; then
    st -d "$CWD" $@ &
else
    st $@ &
fi

If you wanted to use this for a scratchpad then you would want to be able to pass the class or window title to the script, and the script will forward anything you pass to the script over to st via the $@.

So you could do e.g.

~/bin/spawn_cwd.sh -n hello -c myclass

As for scratchpad configuration you'd just change st with the (full?) path to spawn_cwd.sh.

I suppose you could also have it fall back to reading /proc/$PID/cwd if there is no file path in the window title. Less likely that it would make much difference though.

adetabrani commented 8 months ago

1709854094_Mar08_06:28:14

It seems like it almost works, but I'm always in the parent directory of the destination directory when executing the script you provided

here are my sratchpad rule settings

    /* class             role             instance  title                wintype,          tags mask  switchtag   isfloating  alwaysontop isterminal noswallow  floatpos                monitor   scratch key    unmanaged */
    { "cwdterm",         NULL,            NULL,     NULL,                NULL,             0,         0,          1,          0,          1,         0,         "50% 50% 2000W 1200H",  -1,     'c',             0 },

static const char *cwdcmd[] = {"c", "/home/adetabrani/.local/bin/spawn_cwd.sh", "-n", "hello", "-c", "cwdterm", NULL};

static const Key keys[] = {
  { ALTKEY,                       XK_6,      togglescratch,  {.v = cwdcmd } },
    { ALTKEY|ShiftMask,             XK_6,      removescratch,  {.v = cwdcmd } },
    { ALTKEY|ControlMask,           XK_6,      setscratch,     {.v = cwdcmd } },

}