Ticket #1384 (closed enhancement: fixed)

Opened 7 years ago

Last modified 7 years ago

Whitespace highlighting should be optional

Reported by: vda Owned by: angel_il
Priority: major Milestone: 4.7.0-pre1
Component: mcedit Version: master
Keywords: editor highlight trailing space tabs Cc:
Blocked By: Blocking:
Branch state: Votes for changeset: committed-master

Description

I like a new feature of specially displayed tabs and trailing spaces, but sometimes they are getting in the way. For example, when I want to cut-and-paste with the mouse, I usually do not want it.

Also, inevitably there will be users which actively dislike this feature. It's better for the project if it supports those users too.

The most comprehensive solution would be two-fold:

Add a config option for it in F9->Options->General (takes care of those users who hate whitespace highlighting and want it off by default), and

Add a menu item to F9->Edit to toggle whitespace highlighting temporarily with a key combination.

I am setting it as "defect" because technically it is a regression versus 4.6.2: mouse cut-and-paste now is more difficult in some cases.

Attachments

6.patch (7.4 KB) - added by vda 7 years ago.
Fix (diffed against 2-day old git)

Change History

comment:1 in reply to: ↑ description Changed 7 years ago by andrew_b

  • Component changed from mc-core to mcedit
  • Version changed from 4.6.2 to 4.6.2~pre1
  • Type changed from defect to enhancement

Replying to vda:

I like a new feature

Actually, this feature is old :). It was implemented in 4.6.2-pre1. In git:

commit 139e48183f16bc1398d0766d2bcfd7a448227d66
Author: Pavel Tsekov <ptsekov@gmx.net>
Date:   Mon Aug 27 12:06:02 2007 +0000

    * src/color.h: Add a new color pair.
    * src/color.c: Likewise.
    * edit/editdraw.c (print_to_widget): Make tabs and trailing whitespace
    characters visible.
    (edit_draw_this_line): Likewise.

I am setting it as "defect" because technically it is a regression versus 4.6.2

Disagree.

Changed 7 years ago by vda

Fix (diffed against 2-day old git)

comment:2 Changed 7 years ago by angel_il

Branch: 1384_whitespace_highlighting_optional (parent: master)
changeset: 63a3a997db40518d570d7dfce8544dce818f913f

comment:3 Changed 7 years ago by angel_il

  • Version changed from 4.6.2~pre1 to master
  • severity changed from no branch to on review
  • Milestone changed from 4.7 to 4.7.0-pre2

comment:4 Changed 7 years ago by angel_il

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

comment:5 Changed 7 years ago by angel_il

  • Status changed from accepted to assigned

comment:6 follow-up: ↓ 7 Changed 7 years ago by winnie

why are there so many message changes inside the patch? we are not yet in the message freeze but changing so many strings without an ovious need.. (please note that I didn't digg further into the patch only saw the message changes ;-))

comment:7 in reply to: ↑ 6 Changed 7 years ago by andrew_b

  • severity changed from on review to on rework

Replying to winnie:

why are there so many message changes inside the patch?

No. :) Diff looks like the many messages are changed. But actually two new checkboxes are added, therefore some checkboxes change its positions in dialog window.

New checkboxes don't have hotkeys. I think we can fix that in #1381.

Also I'm sure that some optimization of patch is needed. So I change Severity to 'on rework' state.

comment:8 Changed 7 years ago by andrew_b

I thing the current state of tabs and trailing space visualization should be displayed in the editor status bar. For example:

[------] -- tabs and trailing spaces are not shown
[----T-] -- tabs are shown, trailing spaces are not shown
[-----S] -- tabs are not shown, trailing spaces are shown
[----TS] -- tabs and trailing spaces are shown

comment:9 Changed 7 years ago by angel_il

  • severity changed from on rework to on review

changeset: a3dc53022c6fd0ea7d1f00ff755c64fe16601448 (forced update)

comment:10 Changed 7 years ago by angel_il

changeset: dc608c182b77a598c9cb74d24eadd8d16fb78aec (forced update)

comment:11 Changed 7 years ago by slavazanko

  • Votes for changeset set to slavazanko

As for me looks very goog. Nice feature. Remember to add description of new hotkey into our 'docs draft' wiki-area.

comment:12 Changed 7 years ago by angel_il

changeset: 85b8243b475056408da48adddc4983f9143b8923 (forced update)

comment:13 Changed 7 years ago by andrew_b

  • Votes for changeset changed from slavazanko to slavazanko andrew_b
  • severity changed from on review to approved

comment:14 Changed 7 years ago by angel_il

  • Keywords editor highlight trailing space tabs added
  • Status changed from assigned to testing
  • Votes for changeset changed from slavazanko andrew_b to commited-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:15 Changed 7 years ago by angel_il

  • Status changed from testing to closed

comment:16 Changed 7 years ago by slavazanko

  • Milestone changed from 4.7.0-pre2 to 4.7.0-pre1

comment:17 follow-ups: ↓ 18 ↓ 19 Changed 7 years ago by ossi

fwiw, i kinda dislike the complicated shortcut. the idea behind something simple like ctrl-w would have been that i can disable the visualization with the left hand and do the selection with the right hand. as with the current shortcut i have to use both hands, i have to move back my right hand even if it already was on the mouse (which isn't exactly uncommon when i have the need to use the x selection for copying from within an mc window).

comment:18 in reply to: ↑ 17 Changed 7 years ago by angel_il

Replying to ossi:

fwiw, i kinda dislike the complicated shortcut. the idea behind something simple

ok, what shortcut you wish?

comment:19 in reply to: ↑ 17 ; follow-up: ↓ 20 Changed 7 years ago by angel_il

Replying to ossi:

the idea behind something simple

btw, are you can set any key for this, just use ~/.mc/cedit/cooledit.bindings
or try #212 branch

comment:20 in reply to: ↑ 19 ; follow-up: ↓ 21 Changed 7 years ago by ossi

Replying to angel_il:

ok, what shortcut you wish?

well, as i said, i originally thought about ctrl-w (w as in whitespace, and it is right next to ctrl-s which toggles syntax highlighting). anything from qaywsxedc and maybe 1234rfv would be fine with me, obvious collisions excluded. some planning to avoid collisions with likely future functions certainly wouldn't hurt, though.
fwiw, i wouldn't mind if ctrl-s was replaced by something more complex. at least i cannot come up with use cases where i actually need this toggle.

btw, are you can set any key for this, just use ~/.mc/cedit/cooledit.bindings
or try #212 branch

i knew that somebody would say that. but configurability is no excuse for bad defaults.

comment:21 in reply to: ↑ 20 ; follow-up: ↓ 22 Changed 7 years ago by angel_il

Replying to ossi:

Replying to angel_il:

ok, what shortcut you wish?

i knew that somebody would say that. but configurability is no excuse for bad defaults.

ctrl-w maybe... need to discuss it

ok, but think about: this "bad" defaults is bad for you, but not bad for somebody else.

comment:22 in reply to: ↑ 21 Changed 7 years ago by ossi

Replying to angel_il:

ok, but think about: this "bad" defaults is bad for you, but not bad for somebody else.

i think the defaults should be optimized towards right-handers using QWERT style keyboard layouts, simply because that's the biggest target group, and the choice is consistent with the native language of the application. alternative defaults can be relayed to other shortcut schemes.

comment:23 Changed 7 years ago by angel_il

ok. wait pre2...

Note: See TracTickets for help on using tickets.