py3status icon indicating copy to clipboard operation
py3status copied to clipboard

implement proper handling of output_format in py3status for i3bar, dzen2, xmobar, lemonbar, tmux, term, none

Open oaken-source opened this issue 2 years ago • 10 comments

This PR makes py3status compatible with tmux, by implementing the status bar format expected by tmux in addition to the i3 / sway formats already present in py3status.

To enable the tmux format, I extended the domain of the --wm cli parameter to accept 'tmux' as argument.

py3status currently has problems running when directly spawned by tmux non-interactively in tmux.conf, and will cause the CPU usage to spike. this can be prevented by using script like follows:

set -g status-right '#(script -qfec "py3status --wm tmux -c ~/.config/py3status/config-tmux")'

oaken-source avatar Feb 21 '22 11:02 oaken-source

Hi @oaken-source

Thanks for this interesting proposal. As discussed on IRC I'd rather use the output_format which sound more appropriate than diverging from the "wm" terminology.

Would you care doing this instead?

ultrabug avatar Feb 27 '22 16:02 ultrabug

apologies, it seems I completely misunderstood what you were asking on IRC. I agree that adapting the output_format parameter in the config file makes a lot more sense.

oaken-source avatar Feb 27 '22 18:02 oaken-source

rebased to master and applied requested changes. this feature probably also needs a documentation change, but I'm unsure where the right spot would be to add it

oaken-source avatar Feb 27 '22 18:02 oaken-source

please check CI results, mostly black formatting

ultrabug avatar Feb 28 '22 20:02 ultrabug

please check CI results, mostly black formatting

I fixed the issues reported by tox in 7702cf1 -- thanks for pointing that out.

oaken-source avatar Mar 01 '22 17:03 oaken-source

I took a step back, and started looking into i3status first. I created a PR for tmux output in i3status here: https://github.com/i3/i3status/pull/484

oaken-source avatar Mar 05 '22 19:03 oaken-source

Independently of the PR on i3status, it seems to me like the most reasonable way forward would be for py3status to require the value of output_format in general to be i3bar for compatibility with i3status, and introduce another value of output_format in the py3status section of the configuration. alternatively, it would be possible to generate a copy of the user-provided configuration that is passed to i3status, and modify the value of output_format in that config such that all i3status ever sees when called from py3status is a value of i3bar for output_format.

The first way would be easier to implement (add a second output_format setting for py3status) but the second way seems more user-friendly to me (providing i3status with an ephemeral, modified copy of the config file)

oaken-source avatar Mar 05 '22 19:03 oaken-source

this PR now enables full support for the all output_format values supported by i3status (plus tmux) in py3status.

py3status now carries an array of output_format values for which a join by separator is needed before printing the status line. this array is OUTPUT_FORMAT_NEEDS_SEPARATOR and is contained in constants.py. Also, there is a new array in constants.py that contains the default separator for values of output_format that do not default to the string " | ". (currently only dzen2)

This change required that the value of output_format that is passed to the i3status subprocess to be fixed to the value i3bar. This change is made when the temporary config file for the i3status process is created. This allows py3status to correctly parse the output of i3status regardless of the output_format setting. Output formatting is then performed by py3status afterwards.

otherwise, the behaviour of the output formatting is now identical to the i3status output, except for two edge cases:

1.) in the configuration file, a setting of output_format = none will cause i3status to use the none output formatting, while py3status will interpret the value as unset, and default to i3bar instead. to use the none output formatting in py3status, a setting of output_format = "none" is required (needs to be quoted).

2.) i3status uses the separator config setting in the general section to set the separator string between module outputs. Setting this in py3status causes an issue because the config expects separator to be a boolean when looking transitively through the configure options for the modules. Apparently, there is a difference here between the way py3status and i3status handle the configuration files. Note the HACK comment in py3status/module.py.

oaken-source avatar Mar 06 '22 13:03 oaken-source

I agree with the point that these output formatters should probably be object oriented. this was just a quick way to write them very similarly to what is included in i3status, for ease of implementation and a first proof of concept. I'll prepare a new version where this is better encapsulated :)

oaken-source avatar Mar 07 '22 17:03 oaken-source

the latest commit introduces an object oriented approach to the output formatting. it's a lot more code than the if cascades, but probably better decoupled.

oaken-source avatar Mar 07 '22 20:03 oaken-source

@oaken-source getting back to this, I'd like to get it on py3status 3.48

Can you close all conversations you think have been addressed? I tried this PR on my machine and it works fine on i3bar format.

ultrabug avatar Oct 02 '22 15:10 ultrabug

@ultrabug IIRC this looks 99% fine. Can address them later if necessary.

lasers avatar May 31 '23 07:05 lasers

@lasers did you try it yourself? I did a long time ago... maybe I should give it a new try indeed

ultrabug avatar May 31 '23 13:05 ultrabug

Not very recently. I looked at this weeks after this came out. LGTM. I have a diff after this that makes py3status act more like i3status in terminal. You can test this PR for few days too.

lasers avatar May 31 '23 18:05 lasers

fwiw, I'm still using it, but I haven't merged the master in a long while. There is only one unresolved issue, I think, and that's the one above about the output_format parameter being overwritten on the module level, which is not great. but I haven't found a way to fix that yet.

oaken-source avatar May 31 '23 20:05 oaken-source

that's the one above about the output_format parameter being overwritten on the module level

IIRC that's for i3status modules only. AFAIK no i3status module have this config and only one py3status module does (khal_calendar) , but not applicable here so I think it's mostly an non-issue that could be looked again later only when it did happen.

lasers avatar May 31 '23 22:05 lasers

@oaken-source This does it?

diff --git a/py3status/i3status.py b/py3status/i3status.py
index 8fc97e5..c3f6de4 100644
--- a/py3status/i3status.py
+++ b/py3status/i3status.py
@@ -330,10 +330,10 @@ class I3status(Thread):
                         value = TZTIME_FORMAT
                     if key == "format_time":
                         continue
-                # Set output_format to i3bar for parsing regardless of what
-                # formatting we apply ourselves before printing
-                if key == "output_format":
-                    value = "i3bar"
+                # force i3bar output_format in general
+                if section_name == "general":
+                    if key == "output_format":
+                        value = "i3bar"
                 if isinstance(value, bool):
                     value = f"{value}".lower()
                 self.write_in_tmpfile(f'    {key} = "{value}"\n', tmpfile)

lasers avatar Jun 02 '23 11:06 lasers

I rebased to master and applied the change that @lasers suggested above, that should do the trick. Apologies that I didn't get around to this sooner.

Tests look good of course, and I've been running the new build (albeit rebased to 3.50 not master) for a day without issues.

oaken-source avatar Jun 06 '23 07:06 oaken-source

Let's move on and take a leap of faith, thanks a lot for this great work @oaken-source

ultrabug avatar Jun 18 '23 09:06 ultrabug