Dvlv / BoxBuddyRS

A Graphical Interface for Distrobox
MIT License
210 stars 21 forks source link

Adding xfce4-terminal in the list of preferred terminals #73

Closed kadogo closed 4 months ago

kadogo commented 4 months ago

Install method (Flatpak/Binary/Other)

Flatpak

Distrobox Version distrobox --version will tell you this.

1.4.0

Your Host OS

Debian 12 Bookworm

Describe your Issue

Hello, as first thanks for BoxBuddy that I just discovered today. I had issues with DISPLAY not being exported and audio failing. Just using BoxBuddy fix the two of them. I'm curious why, I guess my system doesn't pass some variables by default.

My "issue" here is that I use XFCE, and so I wanted to use xfce4-terminal instead of kconsole that I have installed for various reasons.

It's not a real problem, but if it doesn't make any issue, I think it could be nice to be able to make it work on XFCE out of the box.

Have a good day

Cheers

Dvlv commented 4 months ago

Hello, I wanted to add xfce terminal, but it does not work. See https://dvlv.github.io/BoxBuddyRS/faqs#can-i-use-some_terminal-instead

kadogo commented 4 months ago

Hello @Dvlv

Thank you, sorry I didn't look in the FAQ. I will use konsole it will be fine too ^^

I bit outside this issue, I didn't notice any export of variable in the source, so I don't really explain why it resolves my DISPLAY variable that it was not set by default. If you know why, I'm interested to know for which reason if not it's not an issue I will just be happy that it works on his own ^^

Have a good day

Dvlv commented 4 months ago

Sorry, I think that would be an issue with Distrobox itself, which I'm not able to help with.

kadogo commented 4 months ago

Hello @Dvlv

Sorry to ping you again.

I did a little test by using a wrapper script instead of my konsole binary, and it looks like working. Do you remember what kind of issue you got with xfce4-terminal, so I can maybe do some test?

Here is the wrapper script I use. I'm on Debian 12 so perhaps it can be different on other distributions.

$ cat /usr/bin/konsole
#!/bin/bash

/usr/bin/xfce4-terminal.wrapper -x "$@"
Dvlv commented 4 months ago

The issue with xfce-term is that it expects its command to be a single argument, while all other terminals have the command pieces separate.

e.g.

gnome-terminal -- ls -la 

vs

xfce4-terminal -e "ls -la"

Your wrapper script there will work since you have the $@ in quotes, so the remaining args are made as a single argument.

kadogo commented 4 months ago

Hello @Dvlv

You think it could maybe be used with like a wrapper in flatpak or perhaps a possibility to add a custom command? I think the custom command is potentially better because it gives the possibility to use another script for specific use.

I still add the wrapper of Debian in case it can be related.

#! /usr/bin/perl -w
#
# Terminal.wrapper - Debian terminal wrapper script
#
# Copyright (c) 2004-2005 os-cillation
#

while ($opt = shift(@ARGV))
{
    if ($opt eq '-display')
    {
    $arg = shift(@ARGV);
    push(@args, '--default-display', $arg);
    }
    elsif ($opt eq '-name')
    {
    $arg = shift(@ARGV);
    }
    elsif ($opt eq '-n')
    {
    $arg = shift(@ARGV);
    }
    elsif ($opt eq '-T' || $opt eq '-title')
    {
    push(@args, '--title', shift(@ARGV));
    }
    elsif ($opt eq '-geometry')
    {
    $arg = shift(@ARGV);
    push(@args, "--geometry=$arg");
    }
    elsif ($opt eq '-fn')
    {
    $arg = shift(@ARGV);
    }
    elsif ($opt eq '-fg')
    {
    $arg = shift(@ARGV);
    }
    elsif ($opt eq '-bg')
    {
    $arg = shift(@ARGV);
    }
    elsif ($opt eq '-tn')
    {
    $arg = shift(@ARGV);
    }
    elsif ($opt eq '-e')
    {
    $arg = shift(@ARGV);
    if (@ARGV)
    {
        push(@args, '-x', $arg, @ARGV);
        last;
    }
    else
    {
        push(@args, '-e', $arg);
    }
    last;
    }
    elsif ($opt eq '-h' || $opt eq '--help')
    {
    push(@args, '--help');
    }
}
exec('xfce4-terminal',@args);
Dvlv commented 4 months ago

Thanks, but that feels a bit too hacky. I'd rather keep to the supported terminals.

kadogo commented 4 months ago

Hello @Dvlv

I noticed for example for Konsole you pass -e distrobox enter debian so I did the same for xfce4-terminal and it works but I use -x instead of -e.

$ bash /tmp/b.sh -x distrobox enter debian
$ cat /tmp/b.sh 
xfce4-terminal $@

I will try to make a PR but it looks like the compilation require much free space and for now I don't have enough.

Cheers

Dvlv commented 4 months ago

Wow, -x is working perfectly. Thank you!

Don't worry about making a PR, it's very easy to add another terminal.

Dvlv commented 3 months ago

Xfce terminal support is now added in 2.1.5 Thanks for your investigations!

kadogo commented 3 months ago

Hello @Dvlv

Sorry for not getting back to you sooner. It works like a charm. Thanks again for your time for testing and updating.

Have a good day