After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 724110 - Tidy up profile preferences
Tidy up profile preferences
Status: RESOLVED FIXED
Product: gnome-terminal
Classification: Core
Component: Profiles
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Terminal Maintainers
GNOME Terminal Maintainers
: 660711 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-02-11 11:57 UTC by Allan Day
Modified: 2018-10-19 15:07 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
profile: editor: Remove the word-chars setting (6.24 KB, patch)
2014-04-07 13:31 UTC, Debarshi Ray
accepted-commit_now Details | Review
profile: editor: Remove the word-chars setting (7.18 KB, patch)
2014-04-07 14:00 UTC, Debarshi Ray
committed Details | Review
Remove unused code to create terminals with custom titles (5.48 KB, patch)
2014-04-11 13:53 UTC, Debarshi Ray
committed Details | Review
Remove the action and shortcut to set a static title from the menus (8.67 KB, patch)
2014-04-16 13:08 UTC, Debarshi Ray
committed Details | Review
Remove the static title setting from profile preferences (10.61 KB, patch)
2014-04-16 14:02 UTC, Debarshi Ray
committed Details | Review
screen: Remove unused description and user_title API (5.81 KB, patch)
2014-04-16 15:41 UTC, Debarshi Ray
committed Details | Review
profile: Change the size UI according to latest designs (21.88 KB, patch)
2014-04-17 12:52 UTC, Debarshi Ray
committed Details | Review
Screenshot of the new size UI (39.79 KB, image/png)
2014-04-17 13:01 UTC, Debarshi Ray
  Details
profile: Change the cursor shape UI according to latest designs (13.95 KB, patch)
2014-04-17 13:39 UTC, Debarshi Ray
none Details | Review
Screenshot of the new cursor UI (39.79 KB, image/png)
2014-04-17 13:54 UTC, Debarshi Ray
  Details
profile: Change the cursor shape UI according to latest designs (5.91 KB, patch)
2014-04-28 12:17 UTC, Debarshi Ray
committed Details | Review
Screenshot of the new cursor UI (39.39 KB, image/png)
2014-04-28 12:27 UTC, Debarshi Ray
  Details
profile: Increase the vertical spacing among the rows (1.01 KB, patch)
2014-04-28 12:54 UTC, Debarshi Ray
none Details | Review
profile: Move the terminal bell above the text settings (3.52 KB, patch)
2014-05-02 10:57 UTC, Debarshi Ray
none Details | Review
profile: Move the terminal bell above the text settings (3.89 KB, patch)
2014-05-02 11:41 UTC, Debarshi Ray
none Details | Review
profile: Change the font selection UI according to latest designs (8.80 KB, patch)
2014-05-02 12:49 UTC, Debarshi Ray
none Details | Review
profile: Move the bold and wrap settings above the font button (6.56 KB, patch)
2014-05-02 12:49 UTC, Debarshi Ray
none Details | Review
profile: Add a heading for the text settings (4.48 KB, patch)
2014-05-02 12:50 UTC, Debarshi Ray
none Details | Review
profile: Increase the vertical spacing in the upper rows (4.35 KB, patch)
2014-05-02 12:50 UTC, Debarshi Ray
none Details | Review
Screenshot of the General tab (39.72 KB, image/png)
2014-05-02 12:53 UTC, Debarshi Ray
  Details
profile: Move the terminal bell above the text settings (3.88 KB, patch)
2014-06-30 15:51 UTC, Debarshi Ray
none Details | Review
Screenshot without increase in vertical spacing (40.93 KB, image/png)
2014-06-30 16:01 UTC, Debarshi Ray
  Details
profile: Remove empty space at the top of the General tab (1.03 KB, patch)
2014-06-30 16:15 UTC, Debarshi Ray
none Details | Review
profile: Change the font selection UI according to latest designs (8.80 KB, patch)
2014-07-02 14:18 UTC, Debarshi Ray
none Details | Review
profile: Move the bold and wrap settings above the font button (6.56 KB, patch)
2014-07-02 14:19 UTC, Debarshi Ray
none Details | Review
profile: Add a heading for the text settings (4.48 KB, patch)
2014-07-02 14:19 UTC, Debarshi Ray
none Details | Review
profile: Remove empty space at the top of the General tab (1.03 KB, patch)
2014-07-02 14:20 UTC, Debarshi Ray
none Details | Review
profile: Add some space above the text appearance section (946 bytes, patch)
2014-07-02 14:21 UTC, Debarshi Ray
none Details | Review
profile: Increase the vertical spacing in the upper rows (3.93 KB, patch)
2014-07-02 14:21 UTC, Debarshi Ray
none Details | Review
Screenshot of the General tab (39.32 KB, image/png)
2014-07-08 12:28 UTC, Debarshi Ray
  Details
profile: Move the terminal bell above the text settings (3.89 KB, patch)
2014-07-08 12:30 UTC, Debarshi Ray
committed Details | Review
profile: Change the font selection UI according to latest designs (8.80 KB, patch)
2014-07-08 12:30 UTC, Debarshi Ray
committed Details | Review
profile: Move the bold and wrap settings above the font button (7.80 KB, patch)
2014-07-08 12:31 UTC, Debarshi Ray
none Details | Review
profile: Remove empty space at the top of the General tab (1.03 KB, patch)
2014-07-08 12:32 UTC, Debarshi Ray
committed Details | Review
profile: Don't dim the columns and rows labels (1.54 KB, patch)
2014-07-08 12:33 UTC, Debarshi Ray
committed Details | Review
profile: Add a mnemonic for resetting the size (1.17 KB, patch)
2014-07-08 12:38 UTC, Debarshi Ray
committed Details | Review
profile: Move the bold and wrap settings above the font button (6.55 KB, patch)
2014-07-08 12:52 UTC, Debarshi Ray
committed Details | Review
profile: Add a heading for the text settings (5.65 KB, patch)
2014-07-08 12:52 UTC, Debarshi Ray
committed Details | Review
profile: Move the ID to the second line (2.63 KB, patch)
2014-07-08 13:29 UTC, Debarshi Ray
committed Details | Review
profile: Change the scrollback UI according to latest designs (6.80 KB, patch)
2014-07-09 12:34 UTC, Debarshi Ray
committed Details | Review
profile: Remove useless placeholders and fix the layout (3.15 KB, patch)
2014-07-09 12:35 UTC, Debarshi Ray
none Details | Review
profile: Remove useless placeholders and fix the layout (4.41 KB, patch)
2014-07-09 12:56 UTC, Debarshi Ray
committed Details | Review
Screenshot of the Scrolling tab (22.97 KB, image/png)
2014-07-09 12:56 UTC, Debarshi Ray
  Details

Description Allan Day 2014-02-11 11:57:31 UTC
The preferences could be a lot neater. These mockups try to lay out the controls in a way that looks nicer and is easier to read. They also try to resolve a few minor usability issues with the existing preferences.

https://raw.github.com/gnome-design-team/gnome-mockups/master/terminal/terminal-preferences.png
Comment 1 Christian Persch 2014-04-06 11:38:57 UTC
I guess it's because it's just a mock-up, but it gets capitalisation wrong, missing colons after the labels, and has no mnemonics…

The existing dialogue takes great pain to
* look neat: left justified labels, correct capitalisation, colon at the end, correct alignments, spacings, paddings
* accessible: labels have mnemonics and correctly set-up mnemonic targets

Any redesign must preserve these.

Also, I think a new dialogue should be written from scratch instead of trying to change the .ui file for the current one (glade is just SOOOO unusable!), since it's full of using deprecated widgets like [vh]box, table.


Now let's take on the mockup tab by tab:

* 'general' tab:

- not sure that 'copy ID' is recognisable for what it does

- initial size: currently it's controlled by a checkbox, and if not enabled the size is taken from termcap. Since vte only has one builtin termcap, I guess it's ok to just have this setting directly. Default should be 80x24.

- custom font: Do we need to have this controlled by a checkbox? afaik there's no way to set the default monospace font from the control centre, so maybe just have this as an always-enabled setting?

- terminal bell: NO WAY: I won't accept any GtkSwitch widgets being used in g-t. Should be a checkbox like all the others.

* 'title & command'

- the inline label's not really correct. Do we need this inline help at all? The title set there is a 'static' title that's really only used until the first time the terminal application issues the set-window-title escape sequence... maybe just remove it altogehter?

- run as login shell: seems broken currently (not logged to lastlog)… maybe replace this and the 'custom command' setting with a triple radio:
o Shell
o Login shell
o Custom command: [    ]
?

* 'colours'

- the 'bold text' setting was added at user request, not sure we can just remove it again…

- do we need palette customisation at all? In any case, can't have '[builtin name] *and* customised' at the same time; either it's using a builtin palette or a custom one.

- the inline help text is again not very useful.

* 'scrolling'

- the scrollbar setting already has been changed to only be a checkbox 'use scrollbar'.

- 'unlimited' and 'scrollback size' really are either-or, maybe use a radio group?

* 'compatibility'

- there have just been added 2 more settings

- the 'word chars' setting could just be removed and vte changed to use \w regex instead.
Comment 2 Debarshi Ray 2014-04-07 09:35:38 UTC
(In reply to comment #1)

> * 'colours'
> 
> - the 'bold text' setting was added at user request, not sure we can just
> remove it again…

The 'allow bold text' setting checkbox is still there in 'general'. Or were you talking about something else?
Comment 3 Christian Persch 2014-04-07 09:42:16 UTC
Bold text colour.
Comment 4 Debarshi Ray 2014-04-07 13:23:10 UTC
(In reply to comment #1)

> - the 'word chars' setting could just be removed and vte changed to use \w
> regex instead.

I filed bug 727743 for this one.
Comment 5 Debarshi Ray 2014-04-07 13:31:59 UTC
Created attachment 273706 [details] [review]
profile: editor: Remove the word-chars setting
Comment 6 Christian Persch 2014-04-07 13:52:21 UTC
Comment on attachment 273706 [details] [review]
profile: editor: Remove the word-chars setting

Remove the key from the .gschema.xml file too.
Comment 7 Debarshi Ray 2014-04-07 14:00:28 UTC
Created attachment 273713 [details] [review]
profile: editor: Remove the word-chars setting
Comment 8 Allan Day 2014-04-08 16:20:03 UTC
(In reply to comment #1)
> I guess it's because it's just a mock-up, but it gets capitalisation wrong,
> missing colons after the labels, and has no mnemonics…

OK, I've tidied up the capitalisation and colons. We can easily add mnemonics once the rest of the design has stabilised.

> The existing dialogue takes great pain to
> * look neat: left justified labels, correct capitalisation, colon at the end,
> correct alignments, spacings, paddings

Left justification isn't always the neatest solution (I have some notes on that here: https://wiki.gnome.org/Design/HIG/Layout#Groups_of_Controls ), that said, I don't feel that strongly on this occasion and have updated the mockup to use left justification.

...
> Now let's take on the mockup tab by tab:
> 
> * 'general' tab:
> 
> - not sure that 'copy ID' is recognisable for what it does

Do you think a different label would make it clearer? Since I don't use this feature I'd appreciate some advice on terminology.

...
> - custom font: Do we need to have this controlled by a checkbox? afaik there's
> no way to set the default monospace font from the control centre, so maybe just
> have this as an always-enabled setting?

I think there should be a way to restore the default, in case you change it and can't remember what the default is. The check box works for this and is visually tidy with the items above.

> - terminal bell: NO WAY: I won't accept any GtkSwitch widgets being used in
> g-t. Should be a checkbox like all the others.

I'd like to discuss this with you; the switch works well here in my opinion.

> * 'title & command'
> 
> - the inline label's not really correct. Do we need this inline help at all?
> The title set there is a 'static' title that's really only used until the first
> time the terminal application issues the set-window-title escape sequence...
> maybe just remove it altogehter?

I felt that the purpose of the setting on its own wasn't clear, and the explanation in the mockup was intended as a rough starting point - I'd be perfectly happy to change the text. Removing it is the other option of course; if it doesn't serve a good purpose I'd be happy with that - up to you.

> - run as login shell: seems broken currently (not logged to lastlog)… maybe
> replace this and the 'custom command' setting with a triple radio:
> o Shell
> o Login shell
> o Custom command: [    ]
> ?

Radios make sense here, although I'm not sure "login shell" will be meaningful. Perhaps something like:

o Shell

  [ ] Update login records when shells are launched

o Custom command: [    ]

> * 'colours'
> 
> - the 'bold text' setting was added at user request, not sure we can just
> remove it again…

I moved the bold text setting to the text appearance section in the general tab - since bold text isn't related to colour.

> - do we need palette customisation at all?

I'd be surprised if many people use it, although there's bound to be someone out there. I was just aiming to preserve existing settings.

> In any case, can't have '[builtin
> name] *and* customised' at the same time; either it's using a builtin palette
> or a custom one.

Most custom palettes will be a mix of a predefined palette and a custom one. I think it's important to indicate which predefined palette a custom one is based on, since in many cases only one or two colours will be changed. The palette could be 90% Tango but you'd have no idea.

Also, the current UI changes the unmodified custom palette depending on the previously selected palette - this is non-obvious and it took me some trial and error to figure out where the pre-defined state of the custom palette came from. There's also the oddity of there being two ways to start a custom palette - either by clicking the colour buttons or selecting custom from the drop down.

> - the inline help text is again not very useful.

The intention of the help text was to explain that not all commands use the palette. I have been confused by this in the past - I'd change the palette and see no change in the Terminal output, and I thought it was a bug. Don't mind removing it or changing it though, if you want.

> * 'scrolling'
> 
> - the scrollbar setting already has been changed to only be a checkbox 'use
> scrollbar'.

OK, I've updated the mockup for that.

> - 'unlimited' and 'scrollback size' really are either-or, maybe use a radio
> group?

The idea was that scrollback size would be insensitive if "unlimited" is checked. I've added a note about this to the mockup - let me know if it's OK.

> * 'compatibility'
> 
> - there have just been added 2 more settings

I don't have an up to date build right now; I assume that the extra settings can be incorporated with the mockup; otherwise let me know what the new settings are and I'll add them.
 
> - the 'word chars' setting could just be removed and vte changed to use \w
> regex instead.

Would regex need to be present in the profile preferences?

--

Updated mockups can be found at the original URL: https://raw.github.com/gnome-design-team/gnome-mockups/master/terminal/terminal-preferences.png

Note that the new mockups assume that the settings for terminal titles and word selection will be removed. As a result, the tabs have been reorganised slightly.
Comment 9 Debarshi Ray 2014-04-08 17:51:14 UTC
(In reply to comment #8)
> > - the 'word chars' setting could just be removed and vte changed to use \w
> > regex instead.
> 
> Would regex need to be present in the profile preferences?

Christian meant that we should get rid of the settings and have it built into vte. It has now been removed.
Comment 10 Debarshi Ray 2014-04-11 13:53:28 UTC
Created attachment 274106 [details] [review]
Remove unused code to create terminals with custom titles
Comment 11 Debarshi Ray 2014-04-15 14:03:22 UTC
Review of attachment 274106 [details] [review]:

chpe OKed this in #vte on GIMPNet:

13:46 <rishi> And then one on bug 724110
13:46 <Services> Bug http://bugzilla.gnome.org/show_bug.cgi?id=724110 normal,   
      Normal, gnome-3-14, gnome-terminal-maint, NEW, Tidy up profile            
      preferences
13:49 <chpe> the 'tidy' patch is ok to commit
Comment 12 Debarshi Ray 2014-04-16 13:08:04 UTC
Created attachment 274455 [details] [review]
Remove the action and shortcut to set a static title from the menus
Comment 13 Debarshi Ray 2014-04-16 14:02:55 UTC
Created attachment 274458 [details] [review]
Remove the static title setting from profile preferences
Comment 14 Christian Persch 2014-04-16 14:33:43 UTC
Review of attachment 274458 [details] [review]:

::: src/terminal-screen.c
@@ -729,1 @@
 

I think priv->title is entirely unused with this and the preceding patches, so it can be removed.
Comment 15 Debarshi Ray 2014-04-16 15:41:00 UTC
(In reply to comment #14)
> Review of attachment 274458 [details] [review]:
> 
> ::: src/terminal-screen.c
> @@ -729,1 @@
> 
> 
> I think priv->title is entirely unused with this and the preceding patches, so
> it can be removed.

The whole description and user_title API code can be removed now. I was going to do it in a separate patch, to keep the changes small. I can squash them into one patch too if you prefer it that way.
Comment 16 Debarshi Ray 2014-04-16 15:41:35 UTC
Created attachment 274469 [details] [review]
screen: Remove unused description and user_title API
Comment 17 Debarshi Ray 2014-04-17 08:31:04 UTC
(In reply to comment #8)
> > * 'title & command'
> > 
> > - the inline label's not really correct. Do we need this inline help at all?
> > The title set there is a 'static' title that's really only used until the first
> > time the terminal application issues the set-window-title escape sequence...
> > maybe just remove it altogehter?
> 
> I felt that the purpose of the setting on its own wasn't clear, and the
> explanation in the mockup was intended as a rough starting point - I'd be
> perfectly happy to change the text. Removing it is the other option of course;
> if it doesn't serve a good purpose I'd be happy with that - up to you.

The static title setting has now been removed.
Comment 18 Debarshi Ray 2014-04-17 12:52:37 UTC
Created attachment 274594 [details] [review]
profile: Change the size UI according to latest designs
Comment 19 Debarshi Ray 2014-04-17 13:01:59 UTC
Created attachment 274600 [details]
Screenshot of the new size UI
Comment 20 Debarshi Ray 2014-04-17 13:39:15 UTC
Created attachment 274605 [details] [review]
profile: Change the cursor shape UI according to latest designs
Comment 21 Debarshi Ray 2014-04-17 13:54:08 UTC
Created attachment 274606 [details]
Screenshot of the new cursor UI
Comment 22 Christian Persch 2014-04-17 18:51:44 UTC
Comment on attachment 274605 [details] [review]
profile: Change the cursor shape UI according to latest designs

Not sure about this one. Aligning the right end of the combo with the 'rows' entry above doesn't feel natural, esp since the control above actually is '[ ] cols [ ] rows' ie includes the label...
Comment 23 Christian Persch 2014-04-17 18:52:16 UTC
Since 'title' is gone, should rename the 2nd tab to just 'command'…
Comment 24 Allan Day 2014-04-25 15:19:46 UTC
(In reply to comment #21)
> Created an attachment (id=274606) [details]
> Screenshot of the new cursor UI

Looks like a great start. There are some alignment issues that need tightening up:

 * Need to increase the vertical padding between the rows in the upper section - some elements look too closely related to the controls above and below.

 * "columns" and "rows" are slightly too far from the spin buttons.

 * Putting the font setting on one row (like in the mockup [1]) would use the space more effectively.

 * Can we group the three text settings together, with a heading? (Also like in the mockup.) This will give some much needed structure to this tab. Terminal bell can just go below cursor shape for now.

 * I agree with Christian about the cursor shape combobox - maybe try extending the width so it is the same length as profile name.

[1] https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/terminal/terminal-preferences.png
Comment 25 Christian Persch 2014-04-27 08:34:28 UTC
*** Bug 660711 has been marked as a duplicate of this bug. ***
Comment 26 Debarshi Ray 2014-04-28 12:17:10 UTC
(In reply to comment #24)
>  * "columns" and "rows" are slightly too far from the spin buttons.

Brought them a bit closer to the spin buttons.

>  * I agree with Christian about the cursor shape combobox - maybe try extending
> the width so it is the same length as profile name.

Done.
Comment 27 Debarshi Ray 2014-04-28 12:17:55 UTC
Created attachment 275327 [details] [review]
profile: Change the cursor shape UI according to latest designs
Comment 28 Debarshi Ray 2014-04-28 12:27:26 UTC
Created attachment 275331 [details]
Screenshot of the new cursor UI
Comment 29 Debarshi Ray 2014-04-28 12:54:38 UTC
Created attachment 275341 [details] [review]
profile: Increase the vertical spacing among the rows
Comment 30 Debarshi Ray 2014-04-30 16:01:57 UTC
Review of attachment 275327 [details] [review]:

This was OKed by chpe in #vte on GIMPNet:

13:33 <rishi> chpe: Any comments on bug 724110 ? Just trying to get the         
      patches in one by one so that I don't have to resolve merge conflicts     
      due to rebasing the XML patches.

15:53 <chpe> rishi: well, ok to commit the prefs patch, but I do think the      
      combo is ridiculously wide for these short strings...
Comment 31 Debarshi Ray 2014-05-02 10:57:49 UTC
Created attachment 275631 [details] [review]
profile: Move the terminal bell above the text settings
Comment 32 Christian Persch 2014-05-02 11:08:09 UTC
Comment on attachment 275631 [details] [review]
profile: Move the terminal bell above the text settings

Need to fix the 'position' property of the elements following the place where you removed the checkbutton from, or the file will display wrong in glade.
Comment 33 Debarshi Ray 2014-05-02 11:38:43 UTC
(In reply to comment #32)
> (From update of attachment 275631 [details] [review])
> Need to fix the 'position' property of the elements following the place where
> you removed the checkbutton from, or the file will display wrong in glade.

Hmm... is that so? I have glade-3.16.1 and at appears to be rendering as expected.

According to the XML definition:
  0 - table
  1 - system-font-checkbutton
  2 - alignment containing font-hbox
  3 - allow-bold-checkbutton
  4 - bell-checkbutton (used to be here before being moved to the table)
 10 - rewrap on resize-checkbutton

Looks like glade is able to handle the gap between 4 and 10, but then changing 10 to 5 is trivial, so I will just do that. :)
Comment 34 Debarshi Ray 2014-05-02 11:41:44 UTC
Created attachment 275632 [details] [review]
profile: Move the terminal bell above the text settings
Comment 35 Debarshi Ray 2014-05-02 12:49:09 UTC
Created attachment 275648 [details] [review]
profile: Change the font selection UI according to latest designs
Comment 36 Debarshi Ray 2014-05-02 12:49:46 UTC
Created attachment 275649 [details] [review]
profile: Move the bold and wrap settings above the font button
Comment 37 Debarshi Ray 2014-05-02 12:50:21 UTC
Created attachment 275650 [details] [review]
profile: Add a heading for the text settings
Comment 38 Debarshi Ray 2014-05-02 12:50:58 UTC
Created attachment 275651 [details] [review]
profile: Increase the vertical spacing in the upper rows
Comment 39 Debarshi Ray 2014-05-02 12:53:55 UTC
Created attachment 275652 [details]
Screenshot of the General tab
Comment 40 Debarshi Ray 2014-05-02 12:56:17 UTC
As far as I can make out there is no way to set the xalign of the font_label widget in GtkFontButton from XML. We either need a GTK+ API or we have to write code on our end.
Comment 41 Debarshi Ray 2014-06-30 15:51:01 UTC
Created attachment 279612 [details] [review]
profile: Move the terminal bell above the text settings

Rebased on top of master. No idea how this happened.
Comment 42 Debarshi Ray 2014-06-30 16:01:15 UTC
Created attachment 279615 [details]
Screenshot without increase in vertical spacing
Comment 43 Debarshi Ray 2014-06-30 16:15:53 UTC
Created attachment 279617 [details] [review]
profile: Remove empty space at the top of the General tab
Comment 44 Debarshi Ray 2014-07-02 14:18:36 UTC
Created attachment 279751 [details] [review]
profile: Change the font selection UI according to latest designs
Comment 45 Debarshi Ray 2014-07-02 14:19:20 UTC
Created attachment 279752 [details] [review]
profile: Move the bold and wrap settings above the font button
Comment 46 Debarshi Ray 2014-07-02 14:19:58 UTC
Created attachment 279753 [details] [review]
profile: Add a heading for the text settings
Comment 47 Debarshi Ray 2014-07-02 14:20:37 UTC
Created attachment 279754 [details] [review]
profile: Remove empty space at the top of the General tab
Comment 48 Debarshi Ray 2014-07-02 14:21:13 UTC
Created attachment 279755 [details] [review]
profile: Add some space above the text appearance section
Comment 49 Debarshi Ray 2014-07-02 14:21:50 UTC
Created attachment 279756 [details] [review]
profile: Increase the vertical spacing in the upper rows
Comment 50 Christian Persch 2014-07-08 09:55:31 UTC
I assume the look of the latest patches is like https://bug724110.bugzilla-attachments.gnome.org/attachment.cgi?id=279615 except that the empty space at the top was removed?

Attachment 279756 [details] looks doubly wrong: it uses margins which is not nice, and it increases padding where no padding should increase. Drop this bit, please.

Other nits to fix:
* There needs to be a 18px spacing between "[ ] Terminal bell" and the "Text appearence" header.
* There needs to be a 12px left indent for the items under the "Text appearance" header.
* The "columns" and "rows" labels need to be normal, not dim.

I assume you've checked that all labels still have mnemonics that show up on Alt press, and that activate the correct mnemonic targets?
Comment 51 Debarshi Ray 2014-07-08 12:27:31 UTC
(In reply to comment #50)
> Attachment 279756 [details] looks doubly wrong: it uses margins which is not nice, and it
> increases padding where no padding should increase. Drop this bit, please.

Done.

> Other nits to fix:
> * There needs to be a 18px spacing between "[ ] Terminal bell" and the "Text
> appearence" header.

Done.

> * There needs to be a 12px left indent for the items under the "Text
> appearance" header.

Done.

> * The "columns" and "rows" labels need to be normal, not dim.

Done.
Comment 52 Debarshi Ray 2014-07-08 12:28:46 UTC
Created attachment 280143 [details]
Screenshot of the General tab
Comment 53 Debarshi Ray 2014-07-08 12:30:06 UTC
Created attachment 280144 [details] [review]
profile: Move the terminal bell above the text settings
Comment 54 Debarshi Ray 2014-07-08 12:30:43 UTC
Created attachment 280145 [details] [review]
profile: Change the font selection UI according to latest designs
Comment 55 Debarshi Ray 2014-07-08 12:31:23 UTC
Created attachment 280146 [details] [review]
profile: Move the bold and wrap settings above the font button
Comment 56 Debarshi Ray 2014-07-08 12:32:18 UTC
Created attachment 280147 [details] [review]
profile: Remove empty space at the top of the General tab
Comment 57 Debarshi Ray 2014-07-08 12:33:17 UTC
Created attachment 280148 [details] [review]
profile: Don't dim the columns and rows labels
Comment 58 Debarshi Ray 2014-07-08 12:38:45 UTC
Created attachment 280149 [details] [review]
profile: Add a mnemonic for resetting the size
Comment 59 Debarshi Ray 2014-07-08 12:52:16 UTC
Created attachment 280152 [details] [review]
profile: Move the bold and wrap settings above the font button

Made a mistake during interactively rebasing and squashing.
Comment 60 Debarshi Ray 2014-07-08 12:52:59 UTC
Created attachment 280153 [details] [review]
profile: Add a heading for the text settings
Comment 61 Debarshi Ray 2014-07-08 12:54:10 UTC
These were ACKed by chpe in #vte on GIMPNet:
12:27 <rishi> chpe: Attached a screenshot after addressing your comments.
12:29 <chpe> that looks fine, thanks!
Comment 62 Debarshi Ray 2014-07-08 13:29:26 UTC
Created attachment 280155 [details] [review]
profile: Move the ID to the second line
Comment 63 Debarshi Ray 2014-07-09 12:34:55 UTC
Created attachment 280254 [details] [review]
profile: Change the scrollback UI according to latest designs
Comment 64 Debarshi Ray 2014-07-09 12:35:40 UTC
Created attachment 280255 [details] [review]
profile: Remove useless placeholders and fix the layout
Comment 65 Debarshi Ray 2014-07-09 12:56:11 UTC
Created attachment 280257 [details] [review]
profile: Remove useless placeholders and fix the layout
Comment 66 Debarshi Ray 2014-07-09 12:56:46 UTC
Created attachment 280258 [details]
Screenshot of the Scrolling tab
Comment 67 Debarshi Ray 2014-07-10 12:47:23 UTC
These were ACKed by chpe in #vte on GIMPNet:
12:42 <rishi> chpe: Do you like the scrolling patches?
12:43 <chpe> yes, they're fine
Comment 68 Brian J. Murrell 2015-12-09 13:25:09 UTC
Can I ask why --title was removed?  Was it causing a problem?

Yes, I understand I can use escape sequences to set titles but that is a lot messier than than just being able to do:

$ gnome-terminal --title linux -e "\"screen -x || screen\"

For example.  Now I have to embed escape sequences into all of my command window start ups.

Is my mom (OK, not mom, but equally so a not-a-programmer-user) supposed to know about escape sequences and how to stack/embed them into a terminal startup sequence?  She will remember aptly named command line options much better than she will remember escape sequences.  So will I for that matter.
Comment 69 Brian J. Murrell 2015-12-09 14:30:09 UTC
I would add to my previous comment that requirement of having to echo escape sequences means that every "-e command" gnome-terminal option now has to spawn a shell around the command you would otherwise normally just want to run such as:

$ gnome-terminal -e "bash -c \"echo -en \\\"\033]0;Top\007\\\"; top\""

instead of the previously too simple:

$ gnome-terminal --title Top -e "top"

Could I get rid of the extra bash?  Sure with:

$ gnome-terminal -e "bash -c \"echo -en \\\"\033]0;Top\007\\\"; exec top\""

But is all of this bash trickery with exec and echoing escape sequences and a trip to quote-escaping hell really progress?

Was the simplicity of the "--title" option really so bad to require this alternative?
Comment 71 André Klapper 2018-10-19 15:07:58 UTC
@j.e.labarre: Personal attacks are not acceptable. Read and follow https://wiki.gnome.org/Foundation/CodeOfConduct if you'd like to participate in GNOME spaces. Thanks.