Ticket #2170 (accepted defect) — at Version 18

Opened 10 years ago

Last modified 10 years ago

Color collisions

Reported by: egmont Owned by: slavazanko
Priority: minor Milestone: 4.8.0-pre1
Component: mc-core Version: 4.7.5
Keywords: Cc: gotar@…
Blocked By: Blocking:
Branch state: Votes for changeset: andrew_b

Description (last modified by andrew_b) (diff)

Midnight Commander's color/skin engine often reuses the same color definition for multiple elements on the UI. This unnecessarily limits the possibilities when playing with colors.
See the list below for many examples. Please split all of them into separate color definition keywords. This would allow them to have different colors (and even different attributes, such as underline, if the patch in ticket #2169 gets accepted).
[core]->marked is used in at least three places: for the header of the panels ("'n Name | Size | Modify time"), for files selected with Insert, and for bold font when viewing a preformatted manual page. These three contexts have nothing in common. For the header of the panels, I'd like to have a modest color (probably the same as mc's base colors), because they are always shown and have no reason to be highlighted. Selecting a file is something special, I'd like these selected files to have a really outstanding color with bold font, maybe even a different background. I'd like to have a third pair or colors/attributes for bold in manpages.
The F2 and Alt-e menus use [menu]->menuhot for their title. Dialog and history boxes use [dialog]->dhotnormal. Error boxes use [error]->errdhotnormal for their title. This means if I wish to underline the hotkeys, the title is necessarily underlined, too. In all three cases, the title's color should be a separate new entry, independently of the hotkey colors or any other currently defined colors.
Prompts using the error color scheme (e.g. confirming file deletion) use [error]->_default_ and [error]->errdhotnormal for the non-selected button. For the selected button, the hotkey is colored [error]->errdhotnormal. However, I cannot specify the color for the rest of the selected button: it is always [core]->reverse, the same color as used for the current directory at the very top of the active panel. This is yet again not logical at all, a new color [error]->errdfocus should be used here.
The [caret] symbols to open history are always colored [core]->_default_. In dialogs I'd prefer if I could specify a color matching the dialog's color scheme. Using the panels' main color gives the feeling as if the dialog box was punched here showing whatever's behind.

Change History

Changed 10 years ago by egmont

separate quite a few colors

comment:1 Changed 10 years ago by egmont

The attached patch forks off the following colors:

marked -> header (top of the panels), viewbold (nroff's bold and hex offset)
markselect -> viewselected (in hex mode)
reverse -> errdfocus
dhotnormal -> dtitle
errdhotnormal -> errtitle
helpbold -> helptitle
core/_default_ -> inputhistory, commandhistory (buttons opening the history dialog in the cmd line, and everywhere else)

The patch is designed to be applied in top of the 256color patch (#2169) and mc-4.7.0.5-popup-menu-uses-dialog-colors.patch (#2171). If this latter patch is not accepted by you, then yet another color, the new "menutitle" has to be forked off from "menuhot".

Skins are updated, too. Sand256 is the only one that actually changes.

The only part of the patch that probably needs some explanation is layout.c. I believe that COLOR_HOT_NORMAL there is a leftover that's not needed anymore. update_split(), if decides to output anything, sets the color on its own anyway. And the next character, the "=" sign needs to be printed with normal color.

comment:2 Changed 10 years ago by egmont

Todo:

  • add "old" color name for these
  • update builtin black and white scheme
  • documentation

comment:3 Changed 10 years ago by gotar

  • Cc gotar@… added

Maybe #1670 someone? Any comments there please...

Changed 10 years ago by egmont

update (whitespace/indent changes only)

comment:4 Changed 10 years ago by andrew_b

Unfortunately, mc-4.7.0.6-separate-colors.patch is incomlete:

  • new colors are available via skins only and cannot be set via command line, ini file and environment variale;
  • color help (mc --help-color) is not patched;
  • man page is not patched.

comment:5 Changed 10 years ago by egmont

andrew_b: I'm aware of these, see my earlier comment. This ticket depends on two others, one of them has a patch changing the behavior that I'm not sure you'll accept or reject... I'd pretty much like to see those issues resolved, and then I'll move forward with the missing bits here... Does this sound reasonable?

comment:6 Changed 10 years ago by angel_il

  • Milestone changed from 4.7.3 to 4.7.4

Changed 10 years ago by egmont

update (whitespace/indent changes only)

comment:7 Changed 10 years ago by slavazanko

  • Status changed from new to accepted
  • Owner set to slavazanko
  • severity changed from no branch to on review
  • Milestone changed from 4.7.4 to 4.7.5

Created branch 2170_separate_colors (parent: master)

changeset:9b08798e1c290749c8679f7e2c12945100b10725

Review, please.

comment:8 Changed 10 years ago by slavazanko

New changesets:

Review, please.

comment:9 Changed 10 years ago by slavazanko

ops,

comment:10 Changed 10 years ago by iNode

  • Votes for changeset set to iNode

comment:11 Changed 10 years ago by angel_il

  • Votes for changeset changed from iNode to iNode angel_il
  • severity changed from on review to approved

comment:12 Changed 10 years ago by slavazanko

  • Status changed from accepted to testing
  • Votes for changeset changed from iNode angel_il to commited-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:13 Changed 10 years ago by slavazanko

  • Status changed from testing to closed

comment:14 Changed 10 years ago by andrew_b

  • Status changed from closed to reopened
  • Votes for changeset commited-master deleted
  • severity changed from merged to no branch
  • Priority changed from minor to major
  • Version changed from 4.7.2 to master
  • Resolution fixed deleted

Russian man page is not updated.
Help info is not updated.

comment:15 Changed 10 years ago by slavazanko

  • Status changed from reopened to accepted

comment:16 Changed 10 years ago by slavazanko

  • severity changed from no branch to on review

Created branch 2170_docs:

Review, please.

comment:17 Changed 10 years ago by slavazanko

Well, new changesets in branch:

Chenches in Black&white hardcoded scin not needed.

Review, please.

comment:18 Changed 10 years ago by andrew_b

  • Votes for changeset set to andrew_b
  • Description modified (diff)
Note: See TracTickets for help on using tickets.