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

Implement "title_format" #1747

Merged
merged 3 commits into from Jun 29, 2015
Merged

Implement "title_format" #1747

merged 3 commits into from Jun 29, 2015

Conversation

Airblader
Copy link
Member

I verified that not using a pango font doesn't mistakenly escape the window title. I do strongly suggest we get at least someone else (@acrisci, @ambihelical being canonical candidates) to play around with this first.

One open question for me is if we need to handle other characters when escaping pango markup. The glib implementation seems to do something (see my comment in #1723 where I pasted their implementation), but I don't really understand what's happening there, so input is much appreciated.

Here are some quick screenshots:
screenshot-2015-06-10_19-10-57
screenshot-2015-06-10_19-12-37
screenshot-2015-06-10_19-15-20

fixes #1723

@Airblader
Copy link
Member Author

I completely forgot the documentation. Will add that, of course. :)

@acrisci
Copy link
Member

acrisci commented Jun 10, 2015

I think you should just use the glib implementation.

@Airblader
Copy link
Member Author

You mean just blindly use their logic? I'm not so much a fan of committing code I don't understand.

@acrisci
Copy link
Member

acrisci commented Jun 11, 2015

I think you just made the case against using any libraries at all.

The truth is that nobody understands how computers work.

@Airblader
Copy link
Member Author

But this isn't a library, this is own code that we need to maintain, so we should understand it. Or did you mean pulling in glib as a dependency?

@acrisci
Copy link
Member

acrisci commented Jun 11, 2015

We get glib for free with pango.

@Airblader
Copy link
Member Author

Oh okay. Then it's definitely better than reimplementing this. I'll change it.

@Airblader
Copy link
Member Author

The documentation for g_markup_escape_text contains the explanation, by the way:

Note also that this function will produce character references in
the range of � ... � for all control sequences
except for tabstop, newline and carriage return. The character
references in this range are not valid XML 1.0, but they are
valid XML 1.1 and will be accepted by the GMarkup parser.

@Airblader
Copy link
Member Author

OK, I've switched to the glib implementation and added the documentation.

@ambihelical
Copy link

Nice! I'll try to give it a go this weekend. I still think having some kind of left/right/center region would be useful, and the size balancing not as hard to handle as you have implied. Having the ability to change the appearance of the mark as shown on your window would be one benefit.

@Airblader
Copy link
Member Author

Yeah, I did come around and think alignment options are a superior solution, though we should still define clear behavior about the balancing part upfront. This PR is just the first version of this feature (iterative process), see #1750 for the followup.

@teto
Copy link

teto commented Jun 11, 2015

First of all thanks for working on this and for all the rest of your work. I've tried the examples given in your doc and I succeed in adding a prefix in front of my title with your binding.
But when doing this:
for_window [class="(?i)firefox"] title_format "<span foreground='red'>%title</span>", "%title" is replaced but the is not interpreted and is printed as is. So the firefox title becomes:
"My tab title"
I believe this is a fantastic feature and I am looking forward to the hacks it will create thanks to the IPC, like rainbow tabs :D

@Airblader
Copy link
Member Author

Thanks for the feedback!

but the <span> is not interpreted and is printed as is.

Could you maybe take a screenshot? It works fine for me. Also, can you upload your config somewhere? Finally, please post your reply in the issue, it probably shouldn't be in the PR. Thanks!

like rainbow tabs

I already found myself wanting to play with animations ;)

@ambihelical
Copy link

Feedback; it seems to be working pretty well for me. I tried @teto's example as well and could not reproduce the issue he was having.

One issue I found, with large fonts (size='xx-large') and stacked windows, the text is drawn partially outside of the window title area onto the title area of the window above.

@Airblader
Copy link
Member Author

Thanks for the feedback. I think the issue you mentioned is not something we can reasonably fix.

@teto
Copy link

teto commented Jun 14, 2015

I was surprised by the 4.8 so I rechecked the PR and my git log on the pulled PR is:

634bdcf Added documentation for "title_format".
f177929 Parse the title_format and display the customized window title if a format was set. The format string set with "title_format" 
249af33 Implemented a helper function to escape a string for pango markup.
08d7f23 Added a helper function to decide whether the currently saved font is a pango font.
68cf77d Added command directive 'title_format'. This directive will be used to customize the window title.
1d4b586 Merge pull request #1737 from Airblader/feature-xdotool-on-travis
f262445 Install xdotool to run tests requiring it on Travis.
7b8745f Merge pull request #1735 from shdown/strdup
fe006f0 i3bar: fix freeing static strings
6b505d8 Merge pull request #1733 from Airblader/feature-1732

and
i3 version 4.10.2-193-g634bdcf (2015-06-11, branch "feature-1723") © 2009 Michael Stapelberg and contributors
Are those ok ? because I still have the same problem. If it works for you two I guess it's on my side. In worst case I will wait for a merge but I would be curious to know what's wrong.

@Airblader
Copy link
Member Author

@teto I just checked your config. You're not using a pango font, but rather xft. Of course pango markup cannot work in this case. :)

@teto
Copy link

teto commented Jun 14, 2015

Thanks !! I had no idea what a pango font was (even though you speak of it in yourt first post, shame on me). Now it works as intended ! Once again thanks for your many contributions on i3.

@Airblader
Copy link
Member Author

Great. Thanks for testing this and the feedback.
@stapelberg I think we have enough feedback now.

=== Window title format

By default, i3 will simply print the window's title. Using +title_format+, this can be customized
by setting the format to the desired output. This directive supports pango markup and the following
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should link to pango markup.

@Airblader
Copy link
Member Author

@acrisci Done

int buffer_len = strlen(format) + 1;
for (char *walk = format; *walk != '\0'; walk++) {
if (STARTS_WITH(walk, "%title")) {
buffer_len += strlen(escaped_title);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be buffer_len = buffer_len - strlen("%title") + strlen(escaped_title), shouldn’t it?

@stapelberg
Copy link
Member

Can you please squash the two commits that deal with the escaping function? I.e., I don’t like to have the commit in the history which adds code that is never used and replaced a commit later :).

@stapelberg stapelberg added the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Jun 18, 2015
@Airblader
Copy link
Member Author

I forgot one comment…

@Airblader Airblader added the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Jun 18, 2015
@Airblader
Copy link
Member Author

@stapelberg Adressed the last comment now, too.

@Airblader Airblader removed the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Jun 18, 2015
bindsym $mod+p title_format "Important | %title"

# print all window titles bold
for_window [class="^.*"] title_format "<b>%title</b>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the shortest way to match all windows is for_window [class="."], but I can see how .* might be more familiar, i.e. people might more easily recognize that as a regular expression. The current form of ^.* I don’t quite understand — why the anchoring in the beginning? It doesn’t seem to have any purpose, so please remove it.

@@ -2048,6 +2050,35 @@ Alternatively, if you do not want to mess with +i3-input+, you could create
seperate bindings for a specific set of labels and then only use those labels.
///////////////////////////////////////////////////////////////////

=== Window title format

By default, i3 will simply print the window's title. Using +title_format+, this can be customized
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be good to mention that the application defines the title. In the criteria section of the userguide, we use this terminology:

title
Compares the X11 window title (_NET_WM_NAME or WM_NAME as fallback).

So, I think changing “window's title” to “X11 window title” here would be good for consistency.

@stapelberg stapelberg added the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Jun 29, 2015
This directive will be used to customize the window title.
…ormat was set.

The format string set with "title_format" can contain the placeholder "%title" which will be replaced with the actual window title.

By not overwriting window->name itself, we make sure that assignment matching still works as expected.

fixes i3#1723
@Airblader
Copy link
Member Author

@stapelberg Done.

@Airblader Airblader removed the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Jun 29, 2015
and the following placeholders which will be replaced:

+%title+::
The X11 window title.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should read “The X11 window title (_NET_WM_NAME or WM_NAME as fallback).”.

@stapelberg stapelberg added the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Jun 29, 2015
@Airblader
Copy link
Member Author

@stapelberg But now, hopefully. :)

@Airblader Airblader removed the waiting Waiting for feedback/changes from the author of the issue/PR. Make sure to notify us with a comment. label Jun 29, 2015
stapelberg added a commit that referenced this pull request Jun 29, 2015
@stapelberg stapelberg merged commit 8df7e4e into i3:next Jun 29, 2015
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

Successfully merging this pull request may close these issues.

Support a user-defined title format
5 participants