shlex.split usage for editor config with commands that have spaces

[zmoon]

Bug Report

Related to #581.

Environment

• jrnl --diagnostic output:

  jrnl: v2.6
  Python: 3.9.1 (default, Jan  5 2021, 06:37:06)
  [GCC 5.4.0 20160609]
  OS: Linux 4.4.0-19041-Microsoft
  

• Install method: pipx

Current Behavior vs Expected

I use a long Vim editor string with spaces ("vim -f +Goyo +Limelight \"+set spell linebreak\""). Used to work fine, but in 2.6 it doesn't (some commands are not recognized as commands, instead treats the last command as filename(s)).

Other Information

It looks like in

jrnl/jrnl/editor.py

Line 28 in 61f8406

subprocess.call(shlex.split(config["editor"], posix=on_windows) + [tmpfile])

it should be posix=not on_windows or somesuch, like suggested here. I can confirm that

subprocess.call(shlex.split("vim -f +Goyo +Limelight \"+set spell linebreak\"", posix=True) + ["tmp"])

works fine in my environment, but

subprocess.call(shlex.split("vim -f +Goyo +Limelight \"+set spell linebreak\"", posix=False) + ["tmp"])

produces the same problem that I experience when I invoke jrnl.

[KarimPwnz]

I think this relates to #1096: mainly that shlex.split has weak Windows support.

numpy has dealt with this in numpy/numpy#12979, so we could import their command parser. However, I am not sure if we want to add another dependency.

[zmoon]

@KarimPwnz note that my case here is Linux (though WSL). I did check that on_windows was indeed False. I think in this case posix=True should be passed to shlex.split, instead of posix=False.

[KarimPwnz]

I missed that—you're absolutely right. I was under the assumption that jrnl set posix correctly, and at some point it did: e0177f4. But there was a regression: 631e08a.

[KarimPwnz]

Perhaps fixing that would also fix #1096.

[wren]

Yup! Good catch, both of you!

We should:

• move the shlex calls to a new function (so that it can be tested)
• add a new on_posix function (also so that it can be tested) that probably just returns not on_windows
• change all shlex calls in the code over to new function
• add tests for each function