[REQUEST] Pango markup in -window-format

[jsmnbom]

Before creating a feature request

• I checked the next branch to see if the feature has already been
  implemented
• I searched existing reports to see if it is already requested.

What is the user problem or growth opportunity you want to see solved?

I would like to be able to markup rows in the build in window mode using -window-format. This is already possible using the similar -drun-display-format, so it seems like not implementing it for -window-format might somehow be an oversight?

How do you know that this problem exists today? Why is this important?

I asked on IRC freenode / #rofi and someone there confirmed that -window-format indeed doesn't support pango markup.

Who will benefit from it?

Anyone wanting to spruce up the look of the built in window mode.

Alternate solutions

I think it should be possible to use the script mode of rofi, to reimplment the window switcher, and since that mode supports markup of all rows this issue wouldn't be a problem. Something like https://github.com/lbonn/i3-focus-last might even help, but not quite sure. And anyway, this seems like something that should exist anyhow

Version

Output of rofi -v

Version: 1.6.1

Configuration

Output of rofi -help (in a gist, please paste the full output)
https://gist.github.com/jsmnbom/288445243beb7d9a1be46bcd65d04ffb

Screenshots

Currently it looks like this in window mode

Whereas in drun I can get it to show nicely like this:

If i understand things correctly this should be very similar to what was implemented in 76ceac1, and i might try and PR if this is a feature that indeed should exist, but i know barely any C so not sure how difficult that might be for me.

[DaveDavenport]

Very quick hack:

--- a/source/dialogs/window.c
+++ b/source/dialogs/window.c
@@ -751,11 +751,15 @@ static void helper_eval_add_str ( GString *str, const char *input, int l, int ma
     else {
         if ( nc > l ) {
             int bl = g_utf8_offset_to_pointer ( input_nn, l ) - input_nn;
-            g_string_append_len ( str, input_nn, bl );
+            char *tmp = g_regex_escape_string(input_nn, bl);
+            g_string_append ( str, tmp );
+            g_free(tmp);
         }
         else {
             spaces = l - nc;
-            g_string_append ( str, input_nn );
+            char *tmp = g_regex_escape_string(input_nn, -1);
+            g_string_append ( str, tmp );
+            g_free(tmp);
         }
     }
     while ( spaces-- ) {
@@ -819,6 +823,7 @@ static char *_get_display_value ( const Mode *sw, unsigned int selected_line, in
     if ( c->active ) {
         *state |= ACTIVE;
     }
+    *state |= MARKUP;
     return get_entry ? _generate_display_string ( rmpd, c ) : NULL;
 }

But a better patch can be made, so it supports window managers that use pango markup in workspace name.

[jsmnbom]

Right, so I'm assuming you'd rather not implement the quick hack and instead want a proper fix?

I'm a little confused how other window managers have pango markup in workspace names, and why that would be an issue? Wouldn't the markup from the WM just be passed along, and then this new patch would take care of it?

[DaveDavenport]

we currently strip out the pango markup from that.
So this patch will give decent result, but we can do better.

[jsmnbom]

This isn't very clean, but I assume something like this is what you mean?

--- a/source/dialogs/window.c
+++ b/source/dialogs/window.c
@@ -560,24 +560,28 @@ static void _window_mode_load_data ( Mode *sw, unsigned int cd )
                             char *output = NULL;
                             if ( pango_parse_markup ( _window_name_list_entry ( names.strings, names.strings_len,
                                                                                 c->wmdesktop ), -1, 0, NULL, &output, NULL, NULL ) ) {
-                                c->wmdesktopstr = output;
+                                c->wmdesktopstr = g_strdup ( _window_name_list_entry ( names.strings, names.strings_len, c->wmdesktop ) );
+                                pd->wmdn_len = MAX ( pd->wmdn_len, g_utf8_strlen ( output, -1 ) );
                             }
                             else {
                                 c->wmdesktopstr = g_strdup ( "Invalid name" );
+                                pd->wmdn_len = MAX ( pd->wmdn_len, g_utf8_strlen ( c->wmdesktopstr, -1 ) );
                             }
                         }
                         else {
                             c->wmdesktopstr = g_strdup ( _window_name_list_entry ( names.strings, names.strings_len, c->wmdesktop ) );
+                            pd->wmdn_len = MAX ( pd->wmdn_len, g_utf8_strlen ( c->wmdesktopstr, -1 ) );
                         }
                     }
                     else {
                         c->wmdesktopstr = g_strdup_printf ( "%u", (uint32_t) c->wmdesktop );
+                        pd->wmdn_len = MAX ( pd->wmdn_len, g_utf8_strlen ( c->wmdesktopstr, -1 ) );
                     }
                 }
                 else {
                     c->wmdesktopstr = g_strdup ( "" );
+                    pd->wmdn_len = MAX ( pd->wmdn_len, g_utf8_strlen ( c->wmdesktopstr, -1 ) );
                 }
-                pd->wmdn_len = MAX ( pd->wmdn_len, g_utf8_strlen ( c->wmdesktopstr, -1 ) );
                 if ( cd && c->wmdesktop != current_desktop ) {
                     continue;
                 }

Now looks properly like this, and I also tested with markup in workspace names, though that's not something I use myself.

[DaveDavenport]

hmm my first patch is wrong. Should have been g_markup_escape_text not the regex escape. Now we get errors on & and other characters.

Your patch is indeed one thing that should be changed, but it currently leaks memory and I don't understand why the MAX has been moved and duplicated.

[jsmnbom]

Oh i see, i somehow didn't run into any issues with that.

I know nothing about C so can't say much about the memory leak issue, in terms of why the MAX line is duplicated, is that inside the many ifs, it needs to use the output var, not c->wmdesktopstr, since the c->wmdesktopstr will have all the markup now, and therefore the length will be set wrong. I'm sure there's a cleaner way (perhaps setting a flag if pango_parse_markup returned true, and then checking that flag in the MAX line at the bottom... not sure)

[DaveDavenport]

aah I see about the markup/without markup length. makes sense now.

I think g_free (output) should be added inside the 'pango_parse_markup` block.

we also need to not g_markup_escape the desktopstr if it contains markup, but all other string should be.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.