On 28/10/11 17:56, Chris Larson wrote:
> On Fri, Oct 28, 2011 at 4:42 PM, Joshua Lock <j...@linux.intel.com> wrote:
>> That's Terminal on Fedora and xfce4-terminal on Ubuntu/Debian... This
>> could get interesting!
>>
>> Signed-off-by: Joshua Lock <j...@linux.intel.com>
>> ---
>> meta/lib/oe/terminal.py | 21 +++++++++++++++++++++
>> 1 files changed, 21 insertions(+), 0 deletions(-)
>>
>> diff --git a/meta/lib/oe/terminal.py b/meta/lib/oe/terminal.py
>> index 1455e8e..b90a1f2 100644
>> --- a/meta/lib/oe/terminal.py
>> +++ b/meta/lib/oe/terminal.py
>> @@ -57,6 +57,27 @@ class Gnome(XTerminal):
>> command = 'gnome-terminal --disable-factory -t "{title}" -x {command}'
>> priority = 2
>>
>> +class Xfce(XTerminal):
>> + def distro_name():
>> + import subprocess as sub
>> + try:
>> + p = sub.Popen(['lsb_release', '-i'], stdout=sub.PIPE,
>> + stderr=sub.PIPE)
>> + out, err = p.communicate()
>> + distro = out.split(':').strip()
>> + except:
>> + distro = "unknown"
>> + return distro
>> +
>> + # Upstream binary name is Terminal but Debian/Ubuntu use
>> + # xfce4-terminal to avoid possible(?) conflicts
>> + distro = distro_name()
>> + if distro == 'Ubuntu' or distro == 'Debian':
>> + command = 'xfce4-terminal -T "{title}" -e "{command}"'
>> + else:
>> + command = 'Terminal -T "{title}" -e "{command}"'
>> + priority = 2
>
> The first problem I see with this is you're calling lsb_release at
> module import time. Doing things like that is generally a no-no. I'd
> suggest shifting it into the constructor. Set up the command there as
> self.command, and be sure to call the superclass constructor as well,
> after the command bits. The other thing is, and I'm sure you just
> copied & pasted it from konsole, but we already import a fully
> functional Popen -- it's the superclass of all the terminal classes.
> So there's no need to pull in subprocess's version of it. And the
> defaults of the Popen we use ensure the output is captured, so there's
> no need for the sub.PIPE bits at all if you use that one. Simply do p
> = Popen(['lsb_release', '-i']).
Thanks for the feedback Chris. I'll roll your suggestions into a v2 today.
>
> Thanks for adding this, I'm sure folks using stock Xubuntu and the
> like will find this helpful (I'm an rxvt-unicode guy myself).
I figured that DE would be getting more popular, hence throwing a patch
together.
Cheers,
Joshua
--
Joshua Lock
Yocto Project "Johannes factotum"
Intel Open Source Technology Centre
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
