Skip to content

Commit

Permalink
Improve handling of processes with single argument
Browse files Browse the repository at this point in the history
Previously I would do shlex.split(cmdline[0]) if the cmdline had only
one argument. This would break single argument processes with spaces in
the executable path. Now I check both that the cmdline has one argument
and that that argument is an executable (using shutil.which()). This
means now it will only fail for relative executable paths with spaces in
them, which should be very uncommon.
  • Loading branch information
JonnyHaystack committed Nov 19, 2019
1 parent 0f0a1d1 commit 16a45b7
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 14 deletions.
9 changes: 6 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,12 +477,15 @@ This can be achieved by putting the following in your config file:
### Programs with spaces in the executable path

If the process of a program you are saving has one only argument (the
executable) and the executable path contains spaces, it cannot be saved/restored
correctly unless you [create a custom command mapping](#window-command-mappings)
for it.
executable) and the executable path is a relative path containing spaces, it
cannot be saved/restored correctly unless you
[create a custom command mapping](#window-command-mappings) for it.

See issue #55 for why this is the case.

I think this is a pretty far out edge case though and I'd be surprised if it
caused anyone issues.

### Manually editing programs files

If you manually edit a saved programs file, you must be aware of a few things:
Expand Down
27 changes: 21 additions & 6 deletions i3_resurrect/programs.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import json
import os
import shlex
import shutil
import subprocess
import sys
from pathlib import Path
from distutils import spawn

import i3ipc
import psutil
Expand Down Expand Up @@ -115,10 +118,18 @@ def get_programs(workspace, numeric):
# Get process info for the window.
procinfo = psutil.Process(pid)

# Try to get absolute path to executable.
exe = None
try:
exe = procinfo.exe()
except Exception:
pass

# Create command to launch program.
command = get_window_command(
con['window_properties'],
procinfo.cmdline(),
exe,
)
if command in ([], ''):
continue
Expand Down Expand Up @@ -184,7 +195,7 @@ def get_window_pid(con):
return pid


def get_window_command(window_properties, cmdline):
def get_window_command(window_properties, cmdline, exe):
"""
Gets a window command.
Expand All @@ -194,12 +205,16 @@ def get_window_command(window_properties, cmdline):
"""
window_command_mappings = config.get('window_command_mappings', [])

# If cmdline has only one argument, try to split it. This means we can
# cover cases where the process overwrote its own cmdline, with the
# tradeoff that legitimate single argument cmdlines with spaces in the
# executable path will be broken.
if len(cmdline) == 1:
if len(cmdline) == 1 and shutil.which(cmdline[0]) is None:
# If cmdline has only one argument which is not a known executable
# path, try to split it. This means we can cover cases where the
# process overwrote its own cmdline, with the tradeoff that legitimate
# single argument cmdlines with a relative executable path containing
# spaces will be broken.
cmdline = shlex.split(cmdline[0])
# Use the absolute executable path in case a relative path was used.
if exe is not None:
cmdline[0] = exe

command = cmdline

Expand Down
39 changes: 34 additions & 5 deletions tests/test_programs.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ def test_get_window_command(monkeypatch):
'class': 'Program1',
'title': 'Main window title',
}
assert programs.get_window_command(program1_main, ['program1']) == [
assert programs.get_window_command(
program1_main,
['program1'],
'/usr/bin/program1',
) == [
'run_program1',
]

Expand All @@ -50,22 +54,33 @@ def test_get_window_command(monkeypatch):
'class': 'Program1',
'title': 'Blah random title',
}
assert programs.get_window_command(program1_secondary, ['program1']) == []
assert programs.get_window_command(
program1_secondary,
['program1'],
'/usr/bin/program1',
) == []

# Test with separate program window with matching title but not class.
program2_main = {
'class': 'Program2',
'title': 'Main window title',
}
assert programs.get_window_command(
program2_main, ['program2']) == ['program2']
program2_main,
['program2'],
'/usr/bin/program2'
) == ['/usr/bin/program2']

# Test that title only mapping matches any window with matching title.
program3 = {
'class': 'Program3',
'title': 'Some arbitrary title',
}
assert programs.get_window_command(program3, ['program3']) == []
assert programs.get_window_command(
program3,
['program3'],
'/usr/bin/program3',
) == []

# Test cmdline arg interpolation.
program4 = {
Expand All @@ -75,6 +90,7 @@ def test_get_window_command(monkeypatch):
assert programs.get_window_command(
program4,
['/opt/Program4/program4', '/tmp/test.txt'],
'/opt/Program4/program4',
) == ['run_program4', '/tmp/test.txt']

# Test splitting of single arg command.
Expand All @@ -86,6 +102,7 @@ def test_get_window_command(monkeypatch):
program5,
['/opt/google/chrome/chrome --profile-directory=Default '
'--app=http://instacalc.com --user-data-dir=.config'],
'/opt/google/chrome/chrome',
) == [
'/opt/google/chrome/chrome',
'--profile-directory=Default',
Expand All @@ -102,6 +119,7 @@ def test_get_window_command(monkeypatch):
program6,
['/opt/google/chrome/chrome --profile-directory=Default '
'--app=http://instacalc.com --user-data-dir=.config'],
'/opt/google/chrome/chrome',
) == [
'chrome',
'--profile-directory=Default',
Expand All @@ -113,5 +131,16 @@ def test_get_window_command(monkeypatch):
}
assert programs.get_window_command(
program7,
['/opt/Pulse SMS/pulse-sms']
['/opt/Pulse SMS/pulse-sms'],
None
) == ['/opt/Pulse SMS/pulse-sms']

# Test single arg command with space in executable path with exe available.
program8 = {
'class': 'Program8',
}
assert programs.get_window_command(
program8,
['/opt/Pulse SMS/pulse-sms'],
'/opt/Pulse SMS/pulse-sms',
)

0 comments on commit 16a45b7

Please sign in to comment.