External editor does not work with command line argument

[Dhalgren]

When using an external editor for the "Edit" command (i.e. "Use internal edit" in the Configure Options is unchecked) the environment variable EDITOR is used. However, if $EDITOR contains a command line argument after the executable name, these arguments are not processed properly, and the editor might not be started at all.
How to reproduce:
(Precondition: vi is available on the system)
1.) On the command line, execute: export EDITOR="vi +" && mc
(the + argument should let vi start at the document's end instead of the beginning)
2.) Go to the Options menu -> Configuration -> uncheck "Use internal edit"
3.) Move the cursor to a file that is larger than a single screen (e.g. ABOUT-NLS in mc's source directory)
4.) Press F4 to start the external editor
Result: Nothing visible happens
Expected result: vi is opened showing the end of the file ABOUT-NLS

Output of LC_MESSAGES=C mc -V:

GNU Midnight Commander 4.8.30
Built with GLib 2.78.3
Built with S-Lang 2.3.3 with terminfo database
Built with libssh2 1.11.0
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
With ext2fs attributes support
Virtual File Systems:
 cpiofs, tarfs, sfs, extfs, ftpfs, sftpfs
Data types:
 char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

Output of LC_MESSAGES=C mc -F:

Home directory: /home/user
Profile root directory: /home/user

[System data]
    Config directory: /etc/mc/
    Data directory:   /usr/share/mc/
    File extension handlers: /usr/libexec/mc/ext.d/
    VFS plugins and scripts: /usr/libexec/mc/
	extfs.d:        /usr/libexec/mc/extfs.d/

[User data]
    Config directory: /home/user/.config/mc/
    Data directory:   /home/user/.local/share/mc/
	skins:          /home/user/.local/share/mc/skins/
	extfs.d:        /home/user/.local/share/mc/extfs.d/
	mcedit macros:  /home/user/.local/share/mc/mc.macros
	mcedit external macros: /home/user/.local/share/mc/mcedit/macros.d/macro.*
    Cache directory:  /home/user/.cache/mc/

I have determined the cause of the problem and will provide a fix (which works for a single command line argument) via Pull Request.
Summary: In function utilunix.c:my_system_make_arg_array the GPtrArray storing the contents of argv is not filled correctly.

[Dhalgren]

I see you don't use Pull Requests. I've attached a patch file.

[zaytsev]

Thank you for the patch. Is this function covered by tests? Can you add a test?

[andrew_b]

Unfortunately, the proposed patch is designed for single argument only. If more than one arguments are in $EDITOR: EDITOR="vi -R +", external editor isn't invocated at all.

The full-featured tokenizing in my_system_make_arg_array() is required.

[Dhalgren]

@zaytsev:
It seems that there is a test "execute_execute_external_editor_or_viewer" which pretends to test the function do_executev which subsequently calls the function containing the bug. But actually, only a mock version of do_executev is called which just copies the argv arguments that are passed in. This does not really test the execution of an external editor.
Afterwards it is (erroneously) checked that the arguments are contained in argv starting with index 0 (instead of 1). Of course one could adapt this check, but still the actual call and the fixed code would not be tested.

@andrew_b:
I would say this fix is better than no fix at all. But of course you decide if you want to accept it.

[andrew_b]

Branch: 4533_external_editor_tokens
changeset:44d8213f4ebc16bce3cd68395af885d9e8f3edad

[andrew_b]

Merged to master: [7b3c427c85195603c3a49b66d54cefc2ea0f8450].

[zaytsev]

Thank you very much, Andrew - especially for the tests!