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.
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: emulator rather than isXTerminalEmul ator. /code.launchpad .net/~nfultz/ terminator/ terminator/ +merge/ 163457
> 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_
>
>
> --
> https:/
> You are the owner of lp:~nfultz/terminator/terminator.