Terminator

Merge lp://staging/~nfultz/terminator/terminator into lp://staging/terminator/trunk

terminator
Merge into trunk

Proposed by Neal Fultz on 2013-05-13

Status:

Merged

Merged at revision:

1446

Proposed branch:

lp://staging/~nfultz/terminator/terminator

Merge into:

lp://staging/terminator/trunk

Diff against target:

33 lines (+14/-2)

1 file modified

terminatorlib/optionparse.py (+14/-2)

To merge this branch:

bzr merge lp://staging/~nfultz/terminator/terminator

High

Invalid

Link a bug report

Reviewer	Review Type	Date Requested	Status
Stephen Boddy		2013-05-13	Needs Fixing on 2013-08-04
Review via email: mp+163457@code.staging.launchpad.net

Description of the change

This fixes https://bugs.launchpad.net/terminator/+bug/366644 by switching the -e and -x options when terminator is invoked as x-terminal-emulator.

This is my first bug fix on launchpad :)

Revision history for this message

Stephen Boddy (stephen-j-boddy) wrote on 2013-08-04:

Hi Neal. First, thanks for the contribution. Unfortunately I won't apply this as is. My main issue is that you are changing the behaviour of -x too, by making it refuse arguments. The standard x terminal only has -e, so -x behaviour is not relevant to it. Gnome Terminal does not modify -x behaviour in the wrapper script available. So right now your patch adds more alternative behaviour. I'd be accepting of something that did what the Gnome Terminal wrapper script does (although obviously not in Perl ;-) Either a Python rewrite or integrated into the main terminator script.

Last (minor) point, we don't typically use camel case in the coding of Terminator, so is_X_terminal_emulator rather than isXTerminalEmulator.

review: Needs Fixing

lp://staging/~nfultz/terminator/terminator updated on 2013-08-04

1417. By Neal Fultz <nfultz@neal-1015pe> on 2013-08-04: Bug 366644 / Don't modify -x when invoked as x-terminal-emulator

Revision history for this message

Neal Fultz (nfultz) wrote on 2013-08-04:

OK, pushed a fix; it's a little uglier, though. Just to be explicit,
there are four cases, one of them is supposed to fail:

./terminator -e man htop

terminator: error: Additional unexpected arguments found: ['htop']

./terminator -x man htop

works

./x-terminal-emulator -e man htop

works

./x-terminal-emulator -x man htop

works

On Sun, Aug 04, 2013 at 04:10:33PM -0000, Stephen Boddy wrote:
> Review: Needs Fixing
>
> Hi Neal. First, thanks for the contribution. Unfortunately I won't apply this as is. My main issue is that you are changing the behaviour of -x too, by making it refuse arguments. The standard x terminal only has -e, so -x behaviour is not relevant to it. Gnome Terminal does not modify -x behaviour in the wrapper script available. So right now your patch adds more alternative behaviour. I'd be accepting of something that did what the Gnome Terminal wrapper script does (although obviously not in Perl ;-) Either a Python rewrite or integrated into the main terminator script.
>
> Last (minor) point, we don't typically use camel case in the coding of Terminator, so is_X_terminal_emulator rather than isXTerminalEmulator.
>
>
> --
> https://code.launchpad.net/~nfultz/terminator/terminator/+merge/163457
> You are the owner of lp:~nfultz/terminator/terminator.