Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FR] [quake] add support for role #459

Open
vp1981 opened this issue Jun 24, 2020 · 5 comments
Open

[FR] [quake] add support for role #459

vp1981 opened this issue Jun 24, 2020 · 5 comments

Comments

@vp1981
Copy link

vp1981 commented Jun 24, 2020

Hello,
I propose a patch to support role of window as addition to name but has precedence of it.

diff --git a/util/quake.lua b/util/quake.lua
index 01891b0..39b6ced 100644
--- a/util/quake.lua
+++ b/util/quake.lua
@@ -28,8 +28,12 @@ function quake:display()
     local client = nil
     local i = 0
     for c in awful.client.iterate(function (c)
+      if c.role and self.role then
+	return c.role == self.role
+      else
         -- c.name may be changed!
         return c.instance == self.name
+      end
     end)
     do
         i = i + 1
@@ -119,6 +123,7 @@ function quake:new(config)
 
     conf.app        = conf.app       or "xterm"    -- application to spawn
     conf.name       = conf.name      or "QuakeDD"  -- window name
+    conf.role       = nil                          -- window role
     conf.argname    = conf.argname   or "-name %s" -- how to specify window name
     conf.extra      = conf.extra     or ""         -- extra arguments
     conf.border     = conf.border    or 1          -- client border width

P.S. I didn't test it yet.

@vp1981
Copy link
Author

vp1981 commented Jun 25, 2020

Hello,
I have updated (and now tested) patch:

From 7b30f10c2648ef5d87b1d60a887767296e397ebb Mon Sep 17 00:00:00 2001
From: Vladimir Lomov <[email protected]>
Date: Thu, 25 Jun 2020 17:05:14 +0800
Subject: [PATCH 1/2] util/quake.lua: added support for 'role'

  - role allows to choose a window more granularly than just simple
    'instance' (for example, spacefm set role = 'file_manager' for main
    window and other roles for other windows). Also we use modern way to
    select window --- match from ruled. As I don't understand old
    comment I simply commented old code.

Signed-off-by: Vladimir Lomov <[email protected]>
---
 util/quake.lua | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/util/quake.lua b/util/quake.lua
index 01891b0..b323448 100644
--- a/util/quake.lua
+++ b/util/quake.lua
@@ -8,6 +8,7 @@
 
 local awful        = require("awful")
 local capi         = { client = client }
+local ruled        = require("ruled")
 local math         = math
 local string       = string
 local pairs        = pairs
@@ -27,10 +28,19 @@ function quake:display()
     -- First, we locate the client
     local client = nil
     local i = 0
-    for c in awful.client.iterate(function (c)
-        -- c.name may be changed!
-        return c.instance == self.name
-    end)
+    local client_match = function(c)
+      return ruled.client.match(c, { instance = self.name })
+    end
+    if self.role then
+      client_match = function(c)
+	return ruled.client.match(c, { role = self.role })
+      end
+    end
+    -- for c in awful.client.iterate(function (c)
+    --     -- c.name may be changed!
+    --     return c.instance == self.name
+    -- end)
+    for c in awful.client.iterate(client_match)
     do
         i = i + 1
         if i == 1 then
@@ -119,6 +129,7 @@ function quake:new(config)
 
     conf.app        = conf.app       or "xterm"    -- application to spawn
     conf.name       = conf.name      or "QuakeDD"  -- window name
+    conf.role       = conf.role      or nil        -- window role
     conf.argname    = conf.argname   or "-name %s" -- how to specify window name
     conf.extra      = conf.extra     or ""         -- extra arguments
     conf.border     = conf.border    or 1          -- client border width
-- 
2.27.0

@vp1981
Copy link
Author

vp1981 commented Jun 28, 2020

Hello,
as my main concern was SpaceFM behavior as quake-alike application (see below) I made several patches and tested them on my system. Now I propose two patches:

  1. Patch to use new signal name, it is independent from 'role' patch but second patch has to be applied on top of it
From 496604d148b3b0df4908d7c8913a67b0729c83ae Mon Sep 17 00:00:00 2001
From: Vladimir Lomov <[email protected]>
Date: Fri, 26 Jun 2020 09:42:09 +0800
Subject: [PATCH 1/3] util/quake.lua: use new signal name

Signed-off-by: Vladimir Lomov <[email protected]>
---
 util/quake.lua | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/quake.lua b/util/quake.lua
index 01891b0..27aacfd 100644
--- a/util/quake.lua
+++ b/util/quake.lua
@@ -137,12 +137,12 @@ function quake:new(config)
 
     local dropdown = setmetatable(conf, { __index = quake })
 
-    capi.client.connect_signal("manage", function(c)
+    capi.client.connect_signal("request::manage", function(c)
         if c.instance == dropdown.name and c.screen == dropdown.screen then
             dropdown:display()
         end
     end)
-    capi.client.connect_signal("unmanage", function(c)
+    capi.client.connect_signal("request::unmanage", function(c)
         if c.instance == dropdown.name and c.screen == dropdown.screen then
             dropdown.visible = false
         end
-- 
2.27.0
  1. Patch to add 'role' property
From ae3395e6be3d40b13aabb3d8632798ca4b5563f9 Mon Sep 17 00:00:00 2001
From: Vladimir Lomov <[email protected]>
Date: Sun, 28 Jun 2020 11:19:46 +0800
Subject: [PATCH 2/3] util/quake.lua: added support for role property

  - the 'role' property allows distinguish windows created by an
    application for role they play in application. This patch enables to
    apply 'visibility' to a window with given role.

Signed-off-by: Vladimir Lomov <[email protected]>
---
 util/quake.lua | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/util/quake.lua b/util/quake.lua
index 27aacfd..1945736 100644
--- a/util/quake.lua
+++ b/util/quake.lua
@@ -8,6 +8,7 @@
 
 local awful        = require("awful")
 local capi         = { client = client }
+local ruled        = require("ruled")
 local math         = math
 local string       = string
 local pairs        = pairs
@@ -27,10 +28,19 @@ function quake:display()
     -- First, we locate the client
     local client = nil
     local i = 0
-    for c in awful.client.iterate(function (c)
-        -- c.name may be changed!
-        return c.instance == self.name
-    end)
+    local client_match = function(c)
+      return ruled.client.match(c, { instance = self.name })
+    end
+    if self.role then
+      client_match = function(c)
+	return ruled.client.match(c, { role = self.role })
+      end
+    end
+    -- for c in awful.client.iterate(function (c)
+    --     -- c.name may be changed!
+    --     return c.instance == self.name
+    -- end)
+    for c in awful.client.iterate(client_match)
     do
         i = i + 1
         if i == 1 then
@@ -119,6 +129,7 @@ function quake:new(config)
 
     conf.app        = conf.app       or "xterm"    -- application to spawn
     conf.name       = conf.name      or "QuakeDD"  -- window name
+    conf.role       = conf.role      or nil        -- window role
     conf.argname    = conf.argname   or "-name %s" -- how to specify window name
     conf.extra      = conf.extra     or ""         -- extra arguments
     conf.border     = conf.border    or 1          -- client border width
@@ -138,12 +149,20 @@ function quake:new(config)
     local dropdown = setmetatable(conf, { __index = quake })
 
     capi.client.connect_signal("request::manage", function(c)
-        if c.instance == dropdown.name and c.screen == dropdown.screen then
+        if dropdown.role then
+            if c.role == dropdown.role and c.screen == dropdown.screen then
+              drowdown:display()
+            end
+        elseif c.instance == dropdown.name and c.screen == dropdown.screen then
             dropdown:display()
         end
     end)
     capi.client.connect_signal("request::unmanage", function(c)
-        if c.instance == dropdown.name and c.screen == dropdown.screen then
+        if dropdown.role then
+            if c.role == dropdown.role and c.screen == dropdown.screen then
+                dropdown.visible = false
+            end
+        elseif c.instance == dropdown.name and c.screen == dropdown.screen then
             dropdown.visible = false
         end
      end)
-- 
2.27.0

Now the story. I use SpaceFM as quake-alike application (I like the idea to "hide" application and show it with some keystrokes). But I faced with some issues (before AwesomeWM I used Openbox and didn't see such problems). As SpaceFM window is run as quake-alike application it has predefined size and position but

  1. if I open any dialog window, for example, to rename or delete an item the dialog window inherits the size of "main" window;
  2. and if I abort a dialog window or it finishes it's work the keystroke to "hide" spacefm window didn't work at once, I had to run it twice.

The reason for first issue is that util/quake.lua uses 'instance' that is the same for all spacefm windows, they differ by a 'role' property (I have a patch to add a role for "delete_dialog" as it doesn't have one). The 'role' patch adds support for "role" to "quake" library.

@lcpz
Copy link
Owner

lcpz commented Nov 27, 2020

Hi, sorry for the late reply. I would appreciate if you can turn this into a pull request. Otherwise, I will test myself as soon as I can.

@vp1981
Copy link
Author

vp1981 commented Nov 28, 2020

Hello,
I feel very uncomfortable with github pull requests, I could send patches by email.

@lcpz
Copy link
Owner

lcpz commented Nov 29, 2020

I could send patches by email.

Not necessary, since you have already posted them here.

I will turn them into a new commit myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants