Added format-x support for ipc_module, tested with demo config #2810

madhavpcm · 2022-09-01T19:01:26Z

What type of PR is this? (check all applicable)

Description

Users can now assign separate formats for different hooks inside the IPC module.

Related Issues & Documents

Closes #2775

Documentation (check all applicable)

This PR requires changes to the Wiki documentation (describe the changes)
This PR requires changes to the documentation inside the git repo (please add them to the PR).
Does not require documentation changes

Changes to documentation

In wiki > Modules > ipc
Examples,

[module/demo]
type = custom/ipc
hook-0 = echo foobar
hook-1 = date +%s
hook-2 = whoami
format= <label>
format-0-foreground= ${color.foreground}
format-1-foreground= ${color.foreground}
format-2-foreground= ${color.foreground}
; Based on colors defined by user
format-0-background= ${color.shade1}
format-1-background= ${color.shade2}
format-2-background= ${color.shade3}
initial = 1
click-left = "#demo.hook.0"
click-right = "#demo.hook.1"
double-click-left = "#demo.hook.2"

codecov · 2022-09-01T19:06:22Z

Codecov Report

Merging #2810 (4fc70fa) into master (d817080) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 4fc70fa differs from pull request most recent head 7cf4e9f. Consider uploading reports for the commit 7cf4e9f to get more accurate results

@@            Coverage Diff             @@
##           master    #2810      +/-   ##
==========================================
- Coverage   13.49%   13.47%   -0.02%     
==========================================
  Files         152      151       -1     
  Lines       11422    11515      +93     
==========================================
+ Hits         1541     1552      +11     
- Misses       9881     9963      +82

Flag	Coverage Δ
unittests	`13.47% <0.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/modules/ipc.cpp	`0.00% <0.00%> (ø)`
src/x11/icccm.cpp	`0.00% <0.00%> (ø)`
src/components/bar.cpp	`0.00% <0.00%> (ø)`
src/modules/xwindow.cpp	`0.00% <0.00%> (ø)`
src/modules/backlight.cpp	`0.00% <0.00%> (ø)`
src/components/eventloop.cpp	`0.00% <0.00%> (ø)`
src/components/controller.cpp	`0.00% <0.00%> (ø)`
src/x11/background_manager.cpp	`0.00% <0.00%> (ø)`
include/components/eventloop.hpp	`0.00% <0.00%> (ø)`
include/x11/background_manager.hpp
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

patrick96

Thanks :)

I have unfortunately noticed that this breaks any existing config that does formatting:

[module/ipc]
type = custom/ipc

hook-0 = echo foobar

format-background = #f00
format-foreground = #fff

The current behavior for this is that the module will display foobar with white text and red background.
However, with this PR, those settings are ignored (since we are now using format-0) and the module displays foobar with the bar background and foreground colors.

This makes things more difficult than I first thought. At the very least, we should fall back to format if format-N is not defined.

Please also add a changelog entry for your changes: https://github.com/polybar/polybar/blob/master/CONTRIBUTING.md#changelog

And run clang-format on the files you modified: https://polybar.readthedocs.io/en/latest/dev/style-guide.html#code-formatting

src/modules/ipc.cpp

madhavpcm · 2022-09-02T02:49:25Z

Working on it asap.

patrick96 · 2022-09-02T07:27:20Z

Working on it asap.

There is really no need for stressing about completing this. Take your time, we're not in a hurry :)

madhavpcm · 2022-10-06T11:15:49Z

Thanks :)

I have unfortunately noticed that this breaks any existing config that does formatting:
[module/ipc]
type = custom/ipc

hook-0 = echo foobar

format-background = #f00
format-foreground = #fff
The current behavior for this is that the module will display foobar with white text and red background. However, with this PR, those settings are ignored (since we are now using format-0) and the module displays foobar with the bar background and foreground colors.

Sorry I have been busy last month, Im still stuck on this spot.

A few observations I made, for this config

type = custom/ipc
hook-0 = echo foobar
hook-1 = date +%s
hook-2 = whoami
format= <label>

;format-0 = <label>
;format-1 = <label>
;format-2 = <label>
format-background= #fff
format-foreground= #f22
format-0-foreground= ${color.foreground}
format-1-foreground= ${color.foreground}
format-2-foreground= ${color.foreground}
; Based on colors defined by user
format-0-background= ${color.shade1}
format-1-background= ${color.shade2}
format-2-background= ${color.shade3}
initial = 1
click-left = "#demo.hook.0"
click-right = "#demo.hook.1"
double-click-left = "#demo.hook.2"

add() works without any issue
add_optional() doesnt work because format-X is commented and hence not defined

type = custom/ipc
hook-0 = echo foobar
hook-1 = date +%s
hook-2 = whoami
format= <label>

format-0 = <label>
format-1 = <label>
format-2 = <label>
format-background= #fff
format-foreground= #f22
format-0-foreground= ${color.foreground}
format-1-foreground= ${color.foreground}
format-2-foreground= ${color.foreground}
; Based on colors defined by user
format-0-background= ${color.shade1}
format-1-background= ${color.shade2}
format-2-background= ${color.shade3}
initial = 1
click-left = "#demo.hook.0"
click-right = "#demo.hook.1"
double-click-left = "#demo.hook.2"

add() : formatter gets the value but polybar fails to show the module, no output
add_optional() : formatter gets the value but polybar fails to show the module, no output

Since there is no output, there is no place to trigger other hooks

type = custom/ipc

hook-0 = echo foobar

format-background = #f00
format-foreground = #fff
initial = 1

Works with colors of the main bar, hence what I observed is that we should only call add() if there exists format-x in the config.
For this I have to traverse elements in m_conf->m_sections(m_name) and check if format-x is there, as of now I didnt find any function which does this. But this doesnt explain why there is no output if format-0 is defined as in configuration 2 above.

patrick96 · 2022-10-08T14:15:45Z

I think configuration 2 above is the way to go here. There is no good way to make it work without defininig format-X since its difficult to tell whether any other format properties are defined.

As to why the module doesn't show anything for that, I don't know. If you upload the latest changes, I may be able to take a look.

Works with colors of the main bar, hence what I observed is that we should only call add() if there exists format-x in the config.
For this I have to traverse elements in m_conf->m_sections(m_name) and check if format-x is there, as of now I didnt find any function which does this

The thing you're looking for is m_conf.has. But what you are describing is exactly what add_optional does: it checks if the format exists and only adds it if it does.

madhavpcm · 2022-10-08T15:27:34Z

As to why the module doesn't show anything for that, I don't know. If you upload the latest changes, I may be able to take a look.

Latest changes demonstrate that if format-X is defined, the module just doesnt output anything (as if it crashed). Comment format-0, so it will reach hook-0 output, from there if you trigger any other hook it hides itself.

madhavpcm · 2022-10-08T15:43:58Z

As to why the module doesn't show anything for that, I don't know. If you upload the latest changes, I may be able to take a look.

Latest changes demonstrate that if format-X is defined, the module just doesnt output anything (as if it crashed). Comment format-0, so it will reach hook-0 output, from there if you trigger any other hook it hides itself.
However if you change add_optional to add, and then place this in get format instead

 if (m_conf.has(m_name, format_i)) {
       return DEFAULT_FORMAT;
     } else {
       return format_i;
     }

I notice that, if format is defined, or format-x is defined, and get_format returns the defined format in interest, then the module fails.

patrick96 · 2022-10-08T16:07:09Z

Ah, I see what the issue is. <label> doesn't exist in the ipc module. Try using format-X = <output>

src/modules/ipc.cpp

madhavpcm · 2022-10-08T17:00:12Z

format= <output>
format-background = #ff2
format-0 = <output>
format-1 = <output>
format-2 = <output>

format-0-foreground= ${color.foreground}
format-1-foreground= #f00
format-2-foreground= ${color.foreground}
; Based on colors defined by user
format-0-background= ${color.shade1}
format-2-background= ${color.shade3}

in this particular config,I want say the partial config of format defined here and partial config of a format-x, to be used in the output. However format-1-background is not defined, its falling back to bars default color. Is it possible to mix things up ? I tried looking around the render function calls , didnt notice at first glance.

madhavpcm · 2022-10-09T10:47:48Z

Also, since output is deprecated in some other modules, I can also work on label support for IPC module.

patrick96 · 2022-10-09T13:41:21Z

Is it possible to mix things up ?

No. When any property on a format is not defined, it falls back to how that property is defined in the [settings] section. I think at some point we'll want to have some better fallback mechanisms for formats (there is already an issue about that: #2022), but that would be a bigger undertaking.

Also, since output is deprecated in some other modules, I can also work on label support for IPC module.

That would be cool. I think the ipc module is the only one that doesn't have any labels. But wait until we merged this PR.

patrick96

Alright, this seems to be working now 😃

Plase also add a changelog entry and then we can merge this. https://github.com/polybar/polybar/blob/master/CONTRIBUTING.md#changelog

Added format-x support for ipc_module, tested with demo config

20161c4

patrick96 requested changes Sep 1, 2022

View reviewed changes

src/modules/ipc.cpp Outdated Show resolved Hide resolved

src/modules/ipc.cpp Outdated Show resolved Hide resolved

Certain cases not working

c5d90fc

Certain cases not working, build

8463252

patrick96 reviewed Oct 8, 2022

View reviewed changes

src/modules/ipc.cpp Outdated Show resolved Hide resolved

Changed label to output tag, Mixing of default formats wont work

4fc70fa

patrick96 requested changes Oct 9, 2022

View reviewed changes

created changelog

7cf4e9f

patrick96 approved these changes Oct 9, 2022

View reviewed changes

patrick96 merged commit 54b75f2 into polybar:master Oct 9, 2022

patrick96 mentioned this pull request Nov 5, 2023

Version 3.7.0 #3032

Merged

24 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Added format-x support for ipc_module, tested with demo config #2810

Added format-x support for ipc_module, tested with demo config #2810

Uh oh!

madhavpcm commented Sep 1, 2022 •

edited

Loading

Uh oh!

codecov bot commented Sep 1, 2022 •

edited

Loading

Uh oh!

patrick96 left a comment

Uh oh!

Uh oh!

Uh oh!

madhavpcm commented Sep 2, 2022

Uh oh!

patrick96 commented Sep 2, 2022

Uh oh!

madhavpcm commented Oct 6, 2022

Uh oh!

patrick96 commented Oct 8, 2022

Uh oh!

madhavpcm commented Oct 8, 2022

Uh oh!

madhavpcm commented Oct 8, 2022

Uh oh!

patrick96 commented Oct 8, 2022

Uh oh!

Uh oh!

madhavpcm commented Oct 8, 2022 •

edited

Loading

Uh oh!

madhavpcm commented Oct 9, 2022 •

edited

Loading

Uh oh!

patrick96 commented Oct 9, 2022

Uh oh!

patrick96 left a comment

Uh oh!

Uh oh!

Uh oh!

Added format-x support for ipc_module, tested with demo config #2810

Added format-x support for ipc_module, tested with demo config #2810

Uh oh!

Conversation

madhavpcm commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this? (check all applicable)

Description

Related Issues & Documents

Documentation (check all applicable)

Changes to documentation

Uh oh!

codecov bot commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

patrick96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

madhavpcm commented Sep 2, 2022

Uh oh!

patrick96 commented Sep 2, 2022

Uh oh!

madhavpcm commented Oct 6, 2022

Uh oh!

patrick96 commented Oct 8, 2022

Uh oh!

madhavpcm commented Oct 8, 2022

Uh oh!

madhavpcm commented Oct 8, 2022

Uh oh!

patrick96 commented Oct 8, 2022

Uh oh!

Uh oh!

madhavpcm commented Oct 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madhavpcm commented Oct 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrick96 commented Oct 9, 2022

Uh oh!

patrick96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

madhavpcm commented Sep 1, 2022 •

edited

Loading

codecov bot commented Sep 1, 2022 •

edited

Loading

madhavpcm commented Oct 8, 2022 •

edited

Loading

madhavpcm commented Oct 9, 2022 •

edited

Loading