Ticket #3417 (reopened defect)

Opened 3 years ago

Last modified 3 years ago

Fix vertical line color if a file is marked and selected

Reported by: slavazanko Owned by: slavazanko
Priority: major Milestone: Future Releases
Component: mc-skin Version: master
Keywords: Cc: egmont@…
Blocked By: Blocking:
Branch state: no branch Votes for changeset:

Description

Originally from: https://github.com/MidnightCommander/mc/pull/54

I'm creating my own theme for mc and I noticed that the vertical lines (│) of marked and selected files they had the selected color instead of markselect.

Attachments

54.patch (951 bytes) - added by slavazanko 3 years ago.
mc-3417.png (17.8 KB) - added by egmont 3 years ago.
Regression
mc_without_patch.png (47.3 KB) - added by oblique 3 years ago.
mc_with_patch.png (47.0 KB) - added by oblique 3 years ago.

Change History

Changed 3 years ago by slavazanko

comment:1 Changed 3 years ago by slavazanko

  • Owner set to slavazanko
  • Status changed from new to accepted

comment:2 Changed 3 years ago by slavazanko

  • Votes for changeset set to slavazanko
  • Branch state changed from no branch to approved

comment:3 Changed 3 years ago by slavazanko

  • Status changed from accepted to testing
  • Votes for changeset changed from slavazanko to committed-master
  • Component changed from mc-core to mc-skin
  • Branch state changed from approved to merged
  • Milestone changed from Future Releases to 4.8.14
  • Resolution set to fixed

Merged to master:

git log --pretty=oneline ada7f97...25d554a

comment:4 Changed 3 years ago by slavazanko

  • Status changed from testing to closed

Changed 3 years ago by egmont

Regression

comment:5 Changed 3 years ago by egmont

  • Status changed from closed to reopened
  • Resolution fixed deleted

Wow if only I could type 10 lines of text without the spam detection saying it's spam with 99.8%...

comment:6 Changed 3 years ago by slavazanko

Wow if only I could type 10 lines of text without the spam detection saying it's spam with 99.8%...

Sorry, it's crazy spam-filter :( I've trained the filter so I hope that you'll write comments without any restrictions

Changed 3 years ago by oblique

Changed 3 years ago by oblique

comment:7 Changed 3 years ago by oblique

I don't think that is a regression. IMO the skin you are using is "buggy".

Last edited 3 years ago by oblique (previous) (diff)

comment:8 Changed 3 years ago by slavazanko

I'm really not happy with this change, it breaks quite a few skins, including the default one with TERM=xterm (that is, 256 colors are *not* available and bold+bright is indeed bold). The result is quite ugly as the vertical bar changes to bold, and I don't like its foreground color change either. See the screenshot, it was taken with konsole. (Note: konsole hand-draws the line drawing characters and applies bold to them. The issue might not be visible with other terminals that either take these chars from the font (if the bold font doesn't actually make these bolder), or if the terminal hand-draws these chars but doesn't apply bold, such as gnome-terminal.)

Could you please revert the change (especially if a .14 release is due soon as I suspect)?

We should understand how the said skin wants to look like, and if necessary, maybe introduce a new color for that position and update the existing skins too.

Okay, I reverted these changes. See changeset:cd04e9170d2d18b8c8e88d7782d43685cee94624

comment:9 Changed 3 years ago by slavazanko

  • Milestone changed from 4.8.14 to 4.8.15

comment:10 Changed 3 years ago by oblique

maybe introduce a new color for that position and update the existing skins too.

I like this solution.

comment:11 follow-up: ↓ 13 Changed 3 years ago by oblique

What about introducing these colors: selectedsep, markedsep, markselectsep?

comment:12 Changed 3 years ago by slavazanko

  • Votes for changeset committed-master deleted
  • Branch state changed from merged to no branch

comment:13 in reply to: ↑ 11 Changed 3 years ago by egmont

Replying to oblique:

What about introducing these colors: selectedsep, markedsep, markselectsep?

Sounds good to me.

Old skins should updated too - in the first round in a way that their visual look doesn't change (just copy the colors from the keyword that's used currently); in the second round we could revisit them, maybe some could benefit from the change just like your skin.

I'm not sure if we should wait for it with the 4.8.14 release, main developers will have to decide here. There are one or two other pending tickets about introducing some new color keywords, I think it might make sense to address all of them for 4.8.15.

comment:14 Changed 3 years ago by egmont

  • Cc egmont@… added

comment:15 Changed 3 years ago by andrew_b

  • Milestone changed from 4.8.15 to Future Releases
Note: See TracTickets for help on using tickets.