Added Tmux Selector #1897

StopMotionCuber · 2018-03-23T12:21:34Z

Implementation will show segments depending on whether the current active tmux client is connected via SSH
This still has the flaw that it does not draw independent status bars to the screens, but draws the same depending on the status of the last active pane (whether that has is connected via SSH)
References #1895 and #858

Implementation will show segments depending on whether the current active tmux client is connected via SSH This still has the flaw, that it does not draw independent the screen, but draw the same depending on the status of the last active pane References #1895 and #858

ZyX-I · 2018-03-25T20:48:18Z

powerline/selectors/tmux.py

+def tmux_via_ssh(pl, segment_info, mode, caching=True, default=False):
+	'''Returns True if tmux client is connected via SSH
+	'''
+	pid = get_tmux_output(pl, "display", "-p", "#{client_pid}")


You are not passing environment anywhere, this is not going to work right. The whole bindings file itself was never intended to work in daemon though, so currently you can’t pass environment there.

Is there a better way of doing it?
For me this worked okay-ish (it got the display -p #{client_pid} output for the current active tmux window so that it would not show excluded segments when I was connected via SSH to the machine and working actively on it), also when using the powerline-daemon

You need another tmux instance not run from ssh for that to not be OK. This does not mean that variant is acceptable, you would need to refactor relevant code.

ZyX-I · 2018-03-25T20:50:39Z

powerline/selectors/tmux.py

+		if pid in _cached_clients:
+			return _cached_clients[pid]
+	try:
+		pstree_out = subprocess.check_output(("pstree", "-s", pid)).decode()


Do not use subprocess directly, and do not use check_output at all as it is not present in all supported Python versions. There is run_cmd in powerline.lib.shell for this purpose.

Actually don’t use pstree, as well as any regular expressions. psutil.Process class provides everything needed, it works in-process, it should be more portable and it would not query all the parents, just up to the one that happens to match sshd. Just need to mention requirements in the documentation. pstree is the requirement as well though.

ZyX-I · 2018-03-25T20:55:15Z

powerline/selectors/tmux.py

+_cached_clients = {}
+# This Regex parses pstree -s <Process_ID> information
+# It will match, if any process name is sshd
+SSHD_REGEX = re.compile(r"(?:^|\W)sshd(?:^|\W)")


Most of the strings in powerline use single quotes.

ZyX-I · 2018-03-25T20:56:46Z

powerline/selectors/tmux.py

+SSHD_REGEX = re.compile(r"(?:^|\W)sshd(?:^|\W)")
+
+
+def tmux_via_ssh(pl, segment_info, mode, caching=True, default=False):


Selectors do not take additional arguments, so they must not be present. This will only be confusing in documentation.

ZyX-I · 2018-03-25T21:04:44Z

powerline/selectors/tmux.py

+	if not pid.isnumeric():
+		return default
+	if caching:
+		if pid in _cached_clients:


This logical to write using EAFP principle: just return _cached_clients[pid] and if that throws KeyError proceed with using psutil.

…us though)

StopMotionCuber · 2018-04-17T09:00:00Z

I have done changes so that it works now consistently.
I yet have to add backwards compatibility for tmux < 1.7 and add documentation (including psutil requirement).
For my machine this is working in a nice way. Tell me if you have any concerns about this code

…uced in tmux 2.1)

Added Error Handling for psutil not being installed

StopMotionCuber · 2018-04-20T08:01:00Z

I added documentation now for the tmux SSH selector.
This works like a charm for my tmux version and it should work for every tmux-version >= 2.1.
@ZyX-I I would really appreciate if you could review these changes.

ZyX-I · 2018-04-20T09:44:01Z

powerline/commands/main.py

@@ -107,6 +107,10 @@ def get_argparser(ArgumentParser=argparse.ArgumentParser):
 		'-w', '--width', type=int,
 		help='Maximum prompt with. Triggers truncation of some segments.'
 	)
+	parser.add_argument(
+		'--process-id', metavar='PID', type=int,
+		help='process_ID for the desired process'


Documentation should start with a capital, end with dot, not use identifiers where they are not meant to (what’s wrong with “Process ID”?) and be more descriptive then “desired process”. I have no idea what “desired process” is supposed to mean.

Would 'The process ID of process that renders powerline.' be okay for this?

I think this is fine.

ZyX-I · 2018-04-20T09:52:02Z

powerline/selectors/tmux.py

+
+	Requires the ``psutil`` module.
+
+	Works only for tmux versions >= 2.1'''


This is tied to --process-id being supplied on command-line, not to tmux version in any fashion. Also closing ''' needs to be on its own line and it is not qualified to be a sentence end.

#{client_pid} got introduced with tmux 2.1, so it is not supported before

This is tmux bindings implementation detail. All what selector cares about is whether --process-id is supplied or not, if it is it would be completely happy without even having tmux installed.

ZyX-I · 2018-04-20T09:57:04Z

powerline/selectors/tmux.py

@@ -0,0 +1,34 @@
+# vim:fileencoding=utf-8:noet


And I would actually suggest moving that to powerline/selectors/proc.py and get rid of any references to tmux here. Neither idea of the process tree traversal nor current implementation of the selector is really tmux-specific.

The idea is good, however this requires the exclude function to be named powerline.selectors.proc.is_ssh.
I would propose to change this into a file named common.py and add this module to the general search path for the selectors

ZyX-I · 2018-04-20T09:57:45Z

powerline/selectors/tmux.py

@@ -0,0 +1,34 @@
+# vim:fileencoding=utf-8:noet
+from __future__ import (unicode_literals, division, absolute_import, print_function)
+from powerline.bindings.tmux import get_tmux_output


Unused import.

ZyX-I · 2018-04-20T09:58:31Z

powerline/selectors/tmux.py

+from powerline.bindings.tmux import get_tmux_output
+
+
+_cached_clients = {}


Keys are process identifiers.

ZyX-I · 2018-04-20T09:59:31Z

powerline/selectors/tmux.py

+
+_cached_clients = {}
+
+def tmux_via_ssh(pl, segment_info, mode):


ZyX-I · 2018-04-20T10:00:16Z

powerline/selectors/tmux.py

+_cached_clients = {}
+
+def tmux_via_ssh(pl, segment_info, mode):
+	'''Returns True if tmux client is connected via SSH


… if ``--process-id`` has sshd as its ancestor

ZyX-I · 2018-04-20T10:05:41Z

powerline/selectors/tmux.py

+	try:
+		import psutil
+	except ImportError:
+		pl.warn('Module "psutil" is not installed, thus we cannot detect whether tmux is run via ssh')


This check should be performed at module start. See cpu_load_percent segment in powerline/segments/common/sys: depending on whether import is successfull it either creates an actual class or a dummy which does nothing, but a warning with None return.

ZyX-I · 2018-04-20T10:06:07Z

powerline/selectors/tmux.py

+		return False
+	process_id = segment_info['args'].process_id
+	if process_id is None:
+		pl.warn("Process ID was not passed, maybe you are using tmux version < 2.1")


Remove “, maybe …”.

StopMotionCuber · 2018-04-24T07:49:58Z

Thanks for your Feedback @ZyX-I.
I changed my code according to your requests and did some refactoring to have a common selector module.

ZyX-I · 2018-04-24T08:06:10Z

Do not add second default selectors module. There must not be two default modules out there. Also if you were to do that you also need to

Add tests for new behaviour.
Change linter.
Change documentation.

StopMotionCuber · 2018-04-24T08:38:00Z

The problem with the current approach is that the default module is depending on your current powerline renderer.
For that and my usecase I would have to add it back to tmux.py which would be not portable for other renderers (or they had to add powerline.selectors.tmux.is_ssh in their config. This is too confusing as tmux is not even used).
The other approach would be to always be explicit with powerline.selectors.common.is_ssh as include/exclude_function. I do think this is intuitive for a configuration file either.
If you have a suggestion for a better approach then hit me up, otherwise I will have a look at further needed changes.

ZyX-I · 2018-04-24T09:14:52Z

This is not the first place where path to function is (or needs to be) full with module, and it will not be the last. But AFAIR there are exactly no places where there are two default modules. I would (now) happily remove all default modules completely everywhere, this makes functions easier to discover when needed and configuration file is not a thing that is edited too frequently to bother, but this is backwards incompatible and also not all people share same opinion.

ZyX-I requested changes Mar 25, 2018

View reviewed changes

StopMotionCuber added 4 commits April 17, 2018 08:13

Refactored tmux.py to be more beautiful now (This breaks working stat…

123a5c8

…us though)

Improved selector for tmux via SSH

755bcab

Added default binding for tmux > 1.7 that will include --process-id

8e02b18

Accidentally omitted the -g

f06c10f

StopMotionCuber added 4 commits April 17, 2018 12:53

Removed Changeset from tmux-versions 2.0 (as #{cliend_pid} got introd…

58e7a79

…uced in tmux 2.1)

Reverted 1.7_plus.conf to wrong version -.-

07ed591

Added Tmux Selector documentation

424d1e1

Improved Docstring

1c928df

Added Error Handling for psutil not being installed

ZyX-I reviewed Apr 20, 2018

View reviewed changes

powerline/selectors/tmux.py Outdated

_cached_clients = {}

def tmux_via_ssh(pl, segment_info, mode):

Copy link

Contributor

ZyX-I Apr 20, 2018

Choose a reason for hiding this comment

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

in_ssh.

ZyX-I reviewed Apr 20, 2018

View reviewed changes

Cleaned up code for is_ssh Selector

107a79e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Tmux Selector #1897

Added Tmux Selector #1897

StopMotionCuber commented Mar 23, 2018 •

edited

ZyX-I Mar 25, 2018 •

edited

StopMotionCuber Mar 28, 2018

ZyX-I Mar 28, 2018

ZyX-I Mar 25, 2018

ZyX-I Mar 25, 2018

ZyX-I Mar 25, 2018

ZyX-I Mar 25, 2018

ZyX-I Mar 25, 2018

StopMotionCuber commented Apr 17, 2018

StopMotionCuber commented Apr 20, 2018

ZyX-I Apr 20, 2018

StopMotionCuber Apr 20, 2018

ZyX-I Apr 20, 2018

ZyX-I Apr 20, 2018

StopMotionCuber Apr 20, 2018

ZyX-I Apr 20, 2018

ZyX-I Apr 20, 2018

StopMotionCuber Apr 23, 2018

ZyX-I Apr 20, 2018

ZyX-I Apr 20, 2018

ZyX-I Apr 20, 2018

ZyX-I Apr 20, 2018

ZyX-I Apr 20, 2018

ZyX-I Apr 20, 2018

StopMotionCuber commented Apr 24, 2018

ZyX-I commented Apr 24, 2018

StopMotionCuber commented Apr 24, 2018

ZyX-I commented Apr 24, 2018 •

edited

		SSHD_REGEX = re.compile(r"(?:^\|\W)sshd(?:^\|\W)")


		def tmux_via_ssh(pl, segment_info, mode, caching=True, default=False):


		Requires the ``psutil`` module.

		Works only for tmux versions >= 2.1'''

		from powerline.bindings.tmux import get_tmux_output


		_cached_clients = {}


		_cached_clients = {}

		def tmux_via_ssh(pl, segment_info, mode):

Added Tmux Selector #1897

Are you sure you want to change the base?

Added Tmux Selector #1897

Conversation

StopMotionCuber commented Mar 23, 2018 • edited

ZyX-I Mar 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StopMotionCuber commented Apr 17, 2018

StopMotionCuber commented Apr 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StopMotionCuber commented Apr 24, 2018

ZyX-I commented Apr 24, 2018

StopMotionCuber commented Apr 24, 2018

ZyX-I commented Apr 24, 2018 • edited

StopMotionCuber commented Mar 23, 2018 •

edited

ZyX-I Mar 25, 2018 •

edited

ZyX-I commented Apr 24, 2018 •

edited