Drop xterm for xfce terminal #2208

AlanGriffiths · 2021-08-20T11:59:05Z

No description provided.

townsend2010

Very cool! Need to run clang-format-12 on this:

--- src/platform/client/client_platform_linux.cpp	(before formatting)
+++ src/platform/client/client_platform_linux.cpp	(after formatting)
@@ -26,7 +26,8 @@
 {
     assert(!instance_name.isEmpty() && "Instance name cannot be empty");
     QProcess::startDetached(
-        "x-terminal-emulator", {"--initial-title", instance_name, "-e", QString("bash -c 'multipass shell %1 || read'").arg(instance_name)});
+        "x-terminal-emulator",
+        {"--initial-title", instance_name, "-e", QString("bash -c 'multipass shell %1 || read'").arg(instance_name)});
 }

Saviq · 2021-08-21T16:50:20Z

bors try

codecov · 2021-08-21T17:02:46Z

Codecov Report

Merging #2208 (6793239) into main (6fecaaf) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2208   +/-   ##
=======================================
  Coverage   82.98%   82.98%           
=======================================
  Files         186      186           
  Lines        9725     9725           
=======================================
  Hits         8070     8070           
  Misses       1655     1655

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fecaaf...6793239. Read the comment docs.

Saviq · 2021-08-21T17:34:19Z

Something's wrong with unicode, can anything be done about that?

bors · 2021-08-21T18:16:24Z

try

Build succeeded:

AlanGriffiths · 2021-08-21T21:23:20Z

Something's wrong with unicode, can anything be done about that?

I'd guess that it is a matter of adding a font that supplies the missing codepoints. Will experiment on Monday.

AlanGriffiths · 2021-08-23T08:35:23Z

Another problem (maybe just another manifestation):

$ locale -a
locale: Cannot set LC_CTYPE to default locale: No such file or directory
locale: Cannot set LC_MESSAGES to default locale: No such file or directory
locale: Cannot set LC_COLLATE to default locale: No such file or directory
C
C.UTF-8
POSIX

AlanGriffiths · 2021-08-23T09:35:09Z

Something's wrong with unicode, can anything be done about that?

Just as a sanity check, this wasn't working right with xterm either:

ubuntu@test:~$ snap list
Name       Version   Rev    Tracking       Publisher   Notes
core18     20210722  2128   latest/stable  canonicalâ  base
core20     20210702  1081   latest/stable  canonicalâ  base
lxd        4.0.7     21029  4.0/stable/â¦   canonicalâ  -
multipass  1.7.1     5309   latest/stable  canonicalâ  -
snapd      2.51.3    12704  latest/stable  canonicalâ  snapd

Same example with xfce:

ubuntu@test:~$ snap list
Name       Version   Rev    Tracking       Publisher   Notes
core18     20210722  2128   latest/stable  canonical���  base
core20     20210702  1081   latest/stable  canonical���  base
lxd        4.0.7     21029  4.0/stable/���   canonical���  -
multipass  1.7.1     5309   latest/stable  canonical���  -
snapd      2.51.3    12704  latest/stable  canonical���  snapd

AlanGriffiths · 2021-08-23T10:38:32Z

OK, I finally noticed that I can set the encoding to Unicode [Terminal=>Set encoding=>Unicode] and that works. So, just need to figure out the right incantation.

Saviq · 2021-08-23T15:23:01Z

bors try

bors · 2021-08-23T16:54:56Z

try

Build succeeded:

snap-wrappers/bin/launch-multipass-gui

src/platform/client/client_platform_linux.cpp

Saviq · 2021-08-26T08:54:12Z

bors try

bors · 2021-08-26T08:54:55Z

try

Build failed:

Linux

AlanGriffiths · 2021-08-26T09:01:23Z

FFS that's less readable:

-    QProcess::startDetached(
-        "xfce4-terminal",
-        {"--initial-title", instance_name, "-e", QString("bash -c 'multipass shell %1 || read'").arg(instance_name)});
+    QProcess::startDetached("xfce4-terminal", {"--initial-title", instance_name, "-e",
+                                               QString("bash -c 'multipass shell %1 || read'").arg(instance_name)});

Saviq · 2021-08-26T09:05:22Z

bors try

AlanGriffiths · 2021-08-26T09:15:51Z

@Saviq as you've been testing the earlier version it's worth cleaning out ~/snap/multipass/current/config/xfce4/ to get the new options.

src/platform/client/client_platform_linux.cpp

bors · 2021-08-26T10:36:43Z

try

Build succeeded:

Co-authored-by: Michał Sawicz <[email protected]>

AlanGriffiths · 2021-08-26T10:52:41Z

bors try

bors · 2021-08-26T10:52:41Z

🔒 Permission denied

Existing reviewers: click here to make AlanGriffiths a reviewer

Co-authored-by: Michał Sawicz <[email protected]>

…ipass into xfce-terminal

AlanGriffiths · 2021-08-26T11:14:20Z

bors try

bors · 2021-08-26T13:01:16Z

try

Build failed:

CI (Windows, hyperv)

townsend2010 · 2021-08-26T20:04:34Z

Is it a problem that we can right click on the terminal and open a new tab or new console that essentially escapes the snap confinement? Ideally, that would instead just open another shell into the instance.

Saviq · 2021-08-26T20:39:55Z

Is it a problem that we can right click on the terminal and open a new tab or new console that essentially escapes the snap confinement?

Don't say it's escaping snap confinement. It's not - it's as if you ran snap run --shell multipass.gui.

Ideally, that would instead just open another shell into the instance.

That sounds like heavy customization of the terminal. I would rather we spend the time on running the user's preferred terminal instead.

AlanGriffiths · 2021-08-27T09:32:44Z

Don't say it's escaping snap confinement. It's not - it's as if you ran snap run --shell multipass.gui.

+1

townsend2010 · 2021-08-27T11:51:52Z

Don't say it's escaping snap confinement. It's not - it's as if you ran snap run --shell multipass.gui.

Oh right, it is. Then that's fine.

AlanGriffiths · 2021-08-31T09:03:20Z

bors try

bors · 2021-08-31T10:02:54Z

try

Build failed:

macOS

Saviq

Works fine, handful of tweaks / questions inline.

snap/hooks/post-refresh

snap-wrappers/bin/launch-multipass-gui

Saviq

Thanks!

bors merge

snap-wrappers/bin/launch-multipass-gui

snap/hooks/post-refresh

2208: Drop xterm for xfce terminal r=Saviq a=AlanGriffiths Co-authored-by: Alan Griffiths <[email protected]>

bors · 2021-08-31T15:56:30Z

Build failed:

CI (Windows, hyperv)

Saviq · 2021-08-31T16:48:03Z

bors retry

2208: Drop xterm for xfce terminal r=Saviq a=AlanGriffiths Co-authored-by: Alan Griffiths <[email protected]>

bors · 2021-08-31T18:33:46Z

Build failed:

CI (Windows, hyperv)

AlanGriffiths · 2021-08-31T22:48:59Z

bors retry

bors · 2021-08-31T23:20:47Z

Build failed:

Windows

AlanGriffiths added 3 commits August 20, 2021 12:52

Drop xterm for xfce terminal

a370c76

Drop some unnecessary changes

5fee9f8

Drop some unnecessary changes

f9747d3

townsend2010 reviewed Aug 20, 2021

View reviewed changes

Fix whitespace

a80e7c0

bors bot added a commit that referenced this pull request Aug 21, 2021

Try #2208:

7c55246

Provide a default terminalrc file with Encoding=UTF-8

a85f22e

bors bot added a commit that referenced this pull request Aug 23, 2021

Try #2208:

fd15b14

Saviq requested changes Aug 25, 2021

View reviewed changes

AlanGriffiths added 2 commits August 26, 2021 09:50

Drop unnecessary incantations

1f9743c

Exec xfce4-terminal directly

f381b02

bors bot added a commit that referenced this pull request Aug 26, 2021

Try #2208:

54876d9

placate clang-format

88be2d3

bors bot added a commit that referenced this pull request Aug 26, 2021

Try #2208:

b982afe

Go back to using x-terminal-emulator, but do it better

00d8991

Saviq reviewed Aug 26, 2021

View reviewed changes

src/platform/client/client_platform_linux.cpp Outdated Show resolved Hide resolved

Use -title for the title

85c2cbd

Co-authored-by: Michał Sawicz <[email protected]>

AlanGriffiths and others added 2 commits August 26, 2021 11:58

Use -title for the title

404e3fc

Co-authored-by: Michał Sawicz <[email protected]>

Merge branch 'xfce-terminal' of https://github.com/AlanGriffiths/mult…

0343073

…ipass into xfce-terminal

bors bot added a commit that referenced this pull request Aug 26, 2021

Try #2208:

dea9099

On install/refresh compile the GSettings schemas in the snap

cab9aa8

bors bot added a commit that referenced this pull request Aug 31, 2021

Try #2208:

61fbf5d

Saviq requested changes Aug 31, 2021

View reviewed changes

snap/hooks/post-refresh Outdated Show resolved Hide resolved

snap/hooks/post-refresh Outdated Show resolved Hide resolved

snap-wrappers/bin/launch-multipass-gui Show resolved Hide resolved

Ensure install and post-refresh stay in step

6793239

Saviq approved these changes Aug 31, 2021

View reviewed changes

snap-wrappers/bin/launch-multipass-gui Show resolved Hide resolved

snap/hooks/post-refresh Outdated Show resolved Hide resolved

bors bot added a commit that referenced this pull request Aug 31, 2021

Merge #2208

fe101d2

2208: Drop xterm for xfce terminal r=Saviq a=AlanGriffiths Co-authored-by: Alan Griffiths <[email protected]>

bors bot added a commit that referenced this pull request Aug 31, 2021

Merge #2208

223f659

2208: Drop xterm for xfce terminal r=Saviq a=AlanGriffiths Co-authored-by: Alan Griffiths <[email protected]>

Saviq merged commit 64b8673 into canonical:main Sep 1, 2021

Saviq added this to the v1.8.0 milestone Oct 18, 2021

Drop xterm for xfce terminal #2208

Drop xterm for xfce terminal #2208

Uh oh!

Conversation

AlanGriffiths commented Aug 20, 2021

Uh oh!

townsend2010 left a comment

Choose a reason for hiding this comment

Uh oh!

Saviq commented Aug 21, 2021

Uh oh!

codecov bot commented Aug 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Saviq commented Aug 21, 2021

Uh oh!

bors bot commented Aug 21, 2021

try

Uh oh!

AlanGriffiths commented Aug 21, 2021

Uh oh!

AlanGriffiths commented Aug 23, 2021

Uh oh!

AlanGriffiths commented Aug 23, 2021

Uh oh!

AlanGriffiths commented Aug 23, 2021

Uh oh!

Saviq commented Aug 23, 2021

Uh oh!

bors bot commented Aug 23, 2021

try

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Saviq commented Aug 26, 2021

Uh oh!

bors bot commented Aug 26, 2021

try

Uh oh!

AlanGriffiths commented Aug 26, 2021

Uh oh!

Saviq commented Aug 26, 2021

Uh oh!

AlanGriffiths commented Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bors bot commented Aug 26, 2021

try

Uh oh!

AlanGriffiths commented Aug 26, 2021

Uh oh!

bors bot commented Aug 26, 2021

Uh oh!

AlanGriffiths commented Aug 26, 2021

Uh oh!

bors bot commented Aug 26, 2021

try

Uh oh!

townsend2010 commented Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Saviq commented Aug 26, 2021

Uh oh!

AlanGriffiths commented Aug 27, 2021

Uh oh!

townsend2010 commented Aug 27, 2021

Uh oh!

AlanGriffiths commented Aug 31, 2021

Uh oh!

bors bot commented Aug 31, 2021

try

Uh oh!

Saviq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Aug 21, 2021 •

edited

Loading

AlanGriffiths commented Aug 26, 2021 •

edited

Loading

townsend2010 commented Aug 26, 2021 •

edited

Loading