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

Format tags count towards truncating text #531

Open
YarnSphere opened this issue Apr 12, 2017 · 6 comments
Open

Format tags count towards truncating text #531

YarnSphere opened this issue Apr 12, 2017 · 6 comments
Labels

Comments

@YarnSphere
Copy link

Consider the following module:

[module/test]
type = custom/script
exec = echo "%{B#660000}this text has 27 characters"
label-maxlen = 27

It shows the following:
2017-04-12-191719_111x29_scrot

I believe the format tags shouldn't be considered when truncating the text.

@unode
Copy link

unode commented Apr 20, 2019

Possibly related to this, formatting tags that define actions are disabled on truncated content:

[module/mpd]
type = internal/mpd

label-song = %{A3:sonata:}%artist% - %title%%{A}
label-song-maxlen = 30
label-song-ellipsis = true

Is not clickable and sonata is never launched. However if:

label-song-maxlen = 100

everything works as expected, unless the song title is really long.

I propose relabeling as a bug.

@Lomadriel
Copy link
Member

Yeah indeed, if the string is truncated then the %{A} that close the clickable area is mising and thus ignored the whole action is ignored.
I will try to do something.

@patrick96
Copy link
Member

@Lomadriel what did you have in mind? The only way I think this could be done is by truncating text in the parser instead of when we are generating the intermediate content string

@Lomadriel
Copy link
Member

Lomadriel commented Apr 20, 2019

Hum, maybe I missed something, but this can be solved by modifying this function:
https://github.com/jaagr/polybar/blob/d5112c9b663a3c0d8b013f5110bbf448c18c1ffd/src/components/builder.cpp#L547-L562

If I am not wrong, the only tags that are in the label when this function is called are the formatting ones.
See Lomadriel@b632f66 & Lomadriel@75c712c

@patrick96
Copy link
Member

Well, you'd need to do comprehensive tag parsing inside get_label_text. From a quick glance at your code edge cases like %{A1:echo "}":} and ABC%%{F#ffffff} are not covered.

This would mean we completely parse formatting tags both here and in the parser. We also do parsing of user specified raw tags in the builder currently, but I removed that in 2504f0d because it was buggy.

There is a discussion to be had about whether or not it makes sense to parse tokens both before adding them to the output string and afterwards in the parser. In some cases it might make sense for example here or when fixing #1701. But if we already parse it beforehand it might make sense to actually store the output string in a more structured way that makes the second parsing unnecessary.

@Lomadriel
Copy link
Member

Well, you'd need to do comprehensive tag parsing inside get_label_text. From a quick glance at your code edge cases like %{A1:echo "}":} and ABC%%{F#ffffff} are not covered.

Indeed this is bugged :/

There is a discussion to be had about whether or not it makes sense to parse tokens both before adding them to the output string and afterwards in the parser. In some cases it might make sense for example here or when fixing #1701. But if we already parse it beforehand it might make sense to actually store the output string in a more structured way that makes the second parsing unnecessary.

It might be a good idea to store the output string in a more structured way and could greatly simplify the manipulation of these strings.
If you want I can try to propose an UML for this big rework and so we will be able to discuss that.

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

No branches or pull requests

5 participants