git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gitk: make pointer selection visible in highlighted lines
@ 2013-11-27 18:06 Max Kirillov
  2013-11-27 19:16 ` Thomas Rast
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Max Kirillov @ 2013-11-27 18:06 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git

Custom tags have higher priority than sel, and when they define
their own background, it makes selection invisible. Especially
inconvenient for filesep (to select filenames), but may aslo affect
other tags.

Signed-off-by: Max Kirillov <max@max630.net>
---
 gitk-git/gitk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 5cd00d8..9f350ab 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2385,6 +2385,7 @@ proc makewindow {} {
     $ctext tag conf found -back $foundbgcolor
     $ctext tag conf currentsearchhit -back $currentsearchhitbgcolor
     $ctext tag conf wwrap -wrap word
+    $ctext tag raise sel
 
     .pwbottom add .bleft
     if {!$use_ttk} {
-- 
1.8.4.2.1566.g3c1a064

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] gitk: make pointer selection visible in highlighted lines
  2013-11-27 18:06 [PATCH] gitk: make pointer selection visible in highlighted lines Max Kirillov
@ 2013-11-27 19:16 ` Thomas Rast
  2013-11-27 20:29 ` Eric Sunshine
  2013-11-28 21:20 ` [PATCH v2] " Max Kirillov
  2 siblings, 0 replies; 8+ messages in thread
From: Thomas Rast @ 2013-11-27 19:16 UTC (permalink / raw)
  To: Max Kirillov; +Cc: Paul Mackerras, git

Max Kirillov <max@max630.net> writes:

> Custom tags have higher priority than sel, and when they define
> their own background, it makes selection invisible. Especially
> inconvenient for filesep (to select filenames), but may aslo affect
> other tags.
>
> Signed-off-by: Max Kirillov <max@max630.net>

Nice.

Tested-by: Thomas Rast <tr@thomasrast.ch>

> ---
>  gitk-git/gitk | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 5cd00d8..9f350ab 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -2385,6 +2385,7 @@ proc makewindow {} {
>      $ctext tag conf found -back $foundbgcolor
>      $ctext tag conf currentsearchhit -back $currentsearchhitbgcolor
>      $ctext tag conf wwrap -wrap word
> +    $ctext tag raise sel
>  
>      .pwbottom add .bleft
>      if {!$use_ttk} {

-- 
Thomas Rast
tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gitk: make pointer selection visible in highlighted lines
  2013-11-27 18:06 [PATCH] gitk: make pointer selection visible in highlighted lines Max Kirillov
  2013-11-27 19:16 ` Thomas Rast
@ 2013-11-27 20:29 ` Eric Sunshine
  2013-11-28 21:20 ` [PATCH v2] " Max Kirillov
  2 siblings, 0 replies; 8+ messages in thread
From: Eric Sunshine @ 2013-11-27 20:29 UTC (permalink / raw)
  To: Max Kirillov; +Cc: Paul Mackerras, Git List

On Wed, Nov 27, 2013 at 1:06 PM, Max Kirillov <max@max630.net> wrote:
> Custom tags have higher priority than sel, and when they define
> their own background, it makes selection invisible. Especially
> inconvenient for filesep (to select filenames), but may also affect

s/aslo/also/

> other tags.
>
> Signed-off-by: Max Kirillov <max@max630.net>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] gitk: make pointer selection visible in highlighted lines
  2013-11-27 18:06 [PATCH] gitk: make pointer selection visible in highlighted lines Max Kirillov
  2013-11-27 19:16 ` Thomas Rast
  2013-11-27 20:29 ` Eric Sunshine
@ 2013-11-28 21:20 ` Max Kirillov
  2013-12-01 22:41   ` Paul Mackerras
  2 siblings, 1 reply; 8+ messages in thread
From: Max Kirillov @ 2013-11-28 21:20 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Eric Sunshine, Thomas Rast

Custom tags have higher priority than `sel`, and when they define their
own background, it makes selection invisible. Especially inconvenient
for `filesep` (to select filenames), but also affects other tags.
Use `tag raise` to fix `sel`'s priority.

Also change `omark` tag handling, so that it is created once, together
with others, and then only removed from text rather than deleted. Then
it will not get larger priority than the `sel` tag.

Signed-off-by: Max Kirillov <max@max630.net>
---

Fixed the typo in the comment and selection of text in marked line

 gitk | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gitk b/gitk
index 1dd5137..491a1fa 100755
--- a/gitk
+++ b/gitk
@@ -2029,6 +2029,7 @@ proc makewindow {} {
     global headctxmenu progresscanv progressitem progresscoords statusw
     global fprogitem fprogcoord lastprogupdate progupdatepending
     global rprogitem rprogcoord rownumsel numcommits
+    global markbgcolor
     global have_tk85 use_ttk NS
     global git_version
     global worddiff
@@ -2376,6 +2377,8 @@ proc makewindow {} {
     $ctext tag conf found -back yellow
     $ctext tag conf currentsearchhit -back orange
     $ctext tag conf wwrap -wrap word
+    $ctext tag conf omark -background $markbgcolor
+    $ctext tag raise sel
 
     .pwbottom add .bleft
     if {!$use_ttk} {
@@ -7439,11 +7442,10 @@ proc getblobline {bf id} {
 }
 
 proc mark_ctext_line {lnum} {
-    global ctext markbgcolor
+    global ctext
 
-    $ctext tag delete omark
+    $ctext tag remove omark 1.0 end
     $ctext tag add omark $lnum.0 "$lnum.0 + 1 line"
-    $ctext tag conf omark -background $markbgcolor
     $ctext see $lnum.0
 }
 
-- 
1.8.4.2.1566.g3c1a064

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] gitk: make pointer selection visible in highlighted lines
  2013-11-28 21:20 ` [PATCH v2] " Max Kirillov
@ 2013-12-01 22:41   ` Paul Mackerras
  2013-12-02 10:04     ` Stefan Haller
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Mackerras @ 2013-12-01 22:41 UTC (permalink / raw)
  To: Max Kirillov; +Cc: git, Eric Sunshine, Thomas Rast

On Thu, Nov 28, 2013 at 11:20:18PM +0200, Max Kirillov wrote:
> Custom tags have higher priority than `sel`, and when they define their
> own background, it makes selection invisible. Especially inconvenient
> for `filesep` (to select filenames), but also affects other tags.
> Use `tag raise` to fix `sel`'s priority.
> 
> Also change `omark` tag handling, so that it is created once, together
> with others, and then only removed from text rather than deleted. Then
> it will not get larger priority than the `sel` tag.

This conflicts with the recent change to highlight the current search
hit in orange (c46149, "gitk: Highlight current search hit in
orange").  With the selection being the highest-priority tag, the
current search hit gets shown in grey instead.  I think I prefer the
orange.  I agree though that if the user explicitly selects part of
the text using the mouse, the selection highlight should be the
highest priority in that case.  Maybe the solution is to not select
the search hit automatically?

Paul.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] gitk: make pointer selection visible in highlighted lines
  2013-12-01 22:41   ` Paul Mackerras
@ 2013-12-02 10:04     ` Stefan Haller
  2013-12-03  0:35       ` Max Kirillov
  2013-12-11 23:36       ` [RFC] HACK: use anchor mark instead of sel tag Max Kirillov
  0 siblings, 2 replies; 8+ messages in thread
From: Stefan Haller @ 2013-12-02 10:04 UTC (permalink / raw)
  To: Paul Mackerras, Max Kirillov; +Cc: git, Eric Sunshine, Thomas Rast

Paul Mackerras <paulus@samba.org> wrote:

> On Thu, Nov 28, 2013 at 11:20:18PM +0200, Max Kirillov wrote:
> > Custom tags have higher priority than `sel`, and when they define their
> > own background, it makes selection invisible. Especially inconvenient
> > for `filesep` (to select filenames), but also affects other tags.
> > Use `tag raise` to fix `sel`'s priority.
> > 
> > Also change `omark` tag handling, so that it is created once, together
> > with others, and then only removed from text rather than deleted. Then
> > it will not get larger priority than the `sel` tag.
> 
> This conflicts with the recent change to highlight the current search
> hit in orange (c46149, "gitk: Highlight current search hit in
> orange").  With the selection being the highest-priority tag, the
> current search hit gets shown in grey instead.  I think I prefer the
> orange.  I agree though that if the user explicitly selects part of
> the text using the mouse, the selection highlight should be the
> highest priority in that case.  Maybe the solution is to not select
> the search hit automatically?

I don't think that not selecting the search hint is an option: the
selection is used to keep track of where to search next.

Can't we just raise the currentsearchhit tag above the sel tag?


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] gitk: make pointer selection visible in highlighted lines
  2013-12-02 10:04     ` Stefan Haller
@ 2013-12-03  0:35       ` Max Kirillov
  2013-12-11 23:36       ` [RFC] HACK: use anchor mark instead of sel tag Max Kirillov
  1 sibling, 0 replies; 8+ messages in thread
From: Max Kirillov @ 2013-12-03  0:35 UTC (permalink / raw)
  To: Paul Mackerras, Stefan Haller; +Cc: git, Eric Sunshine, Thomas Rast

On Mon, Dec 02, 2013 at 11:04:09AM +0100, Stefan Haller wrote:
> I don't think that not selecting the search hint is an
> option: the selection is used to keep track of where to
> search next.

To mark the next found position, should a 0-length selection
be enough? I will try to experiment with it.

> Can't we just raise the currentsearchhit tag above the sel
> tag?

This also seems to help, as far as I understod
currentsearchhit is always removed as interactive selection
starts, so it should not hide manual selection.

-- 
Max

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC] HACK: use anchor mark instead of sel tag
  2013-12-02 10:04     ` Stefan Haller
  2013-12-03  0:35       ` Max Kirillov
@ 2013-12-11 23:36       ` Max Kirillov
  1 sibling, 0 replies; 8+ messages in thread
From: Max Kirillov @ 2013-12-11 23:36 UTC (permalink / raw)
  To: Stefan Haller; +Cc: Paul Mackerras, git

---

I hacked somehow around this. It seems that just usilg the
anchor mark should be enough to implement almost the same
behavior. The hard part is that I don't know which feature
is intentional and which is just random consequence of using
sel for search highlight.

One thing which seems to me important and not possible to
implement without using another mask or tag is to return
currentsearchhit back when user removes characters from the
end of search string, and it starts to match previous
places.

This is not intended as patch for merging, just proof of
concept.

 gitk-git/gitk | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index cf25472..c8b223f 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -8320,24 +8320,29 @@ proc settabs {{firstab {}}} {
 proc incrsearch {name ix op} {
     global ctext searchstring searchdirn
 
-    if {[catch {$ctext index anchor}]} {
+    if {[catch {set prev_anchor [$ctext index anchor]}]} {
 	# no anchor set, use start of selection, or of visible area
 	set sel [$ctext tag ranges sel]
 	if {$sel ne {}} {
-	    $ctext mark set anchor [lindex $sel 0]
+	    set start [lindex $sel 0]
 	} elseif {$searchdirn eq "-forwards"} {
-	    $ctext mark set anchor @0,0
+	    set start @0,0
+	} else {
+	    set start @0,[winfo height $ctext]
+	}
+    } else {
+	if {$searchdirn eq "-forwards"} {
+	    set start $prev_anchor
 	} else {
-	    $ctext mark set anchor @0,[winfo height $ctext]
+	    set start "$prev_anchor + [string length $searchstring] c"
 	}
     }
     if {$searchstring ne {}} {
-	set here [$ctext search -count mlen $searchdirn -- $searchstring anchor]
+	set here [$ctext search -count mlen $searchdirn -- $searchstring $start]
 	if {$here ne {}} {
 	    $ctext see $here
 	    set mend "$here + $mlen c"
-	    $ctext tag remove sel 1.0 end
-	    $ctext tag add sel $here $mend
+	    $ctext mark set anchor $here
 	    suppress_highlighting_file_for_current_scrollpos
 	    highlightfile_for_scrollpos $here
 	}
@@ -8355,7 +8360,7 @@ proc dosearch {} {
 	set sel [$ctext tag ranges sel]
 	if {$sel ne {}} {
 	    set start "[lindex $sel 0] + 1c"
-	} elseif {[catch {set start [$ctext index anchor]}]} {
+	} elseif {[catch {set start "[$ctext index anchor] + 1c"}]} {
 	    set start "@0,0"
 	}
 	set match [$ctext search -count mlen -- $searchstring $start]
@@ -8368,8 +8373,7 @@ proc dosearch {} {
 	suppress_highlighting_file_for_current_scrollpos
 	highlightfile_for_scrollpos $match
 	set mend "$match + $mlen c"
-	$ctext tag add sel $match $mend
-	$ctext mark unset anchor
+	$ctext mark set anchor $match
 	rehighlight_search_results
     }
 }
@@ -8397,8 +8401,7 @@ proc dosearchback {} {
 	suppress_highlighting_file_for_current_scrollpos
 	highlightfile_for_scrollpos $match
 	set mend "$match + $ml c"
-	$ctext tag add sel $match $mend
-	$ctext mark unset anchor
+        $ctext mark set anchor $match
 	rehighlight_search_results
     }
 }
@@ -8418,14 +8421,15 @@ proc searchmark {first last} {
     global ctext searchstring
     puts [list $first $last]
 
-    set sel [$ctext tag ranges sel]
+    #TODO: catch no anchor
+    set anchor [$ctext index anchor]
 
     set mend $first.0
     while {1} {
 	set match [$ctext search -count mlen -- $searchstring $mend $last.end]
 	if {$match eq {}} break
 	set mend "$match + $mlen c"
-	if {$sel ne {} && [$ctext compare $match == [lindex $sel 0]]} {
+	if {$anchor eq $match} {
 	    $ctext tag add currentsearchhit $match $mend
 	} else {
 	    $ctext tag add found $match $mend
-- 
1.8.4.2.1566.g3c1a064

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-12-11 23:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 18:06 [PATCH] gitk: make pointer selection visible in highlighted lines Max Kirillov
2013-11-27 19:16 ` Thomas Rast
2013-11-27 20:29 ` Eric Sunshine
2013-11-28 21:20 ` [PATCH v2] " Max Kirillov
2013-12-01 22:41   ` Paul Mackerras
2013-12-02 10:04     ` Stefan Haller
2013-12-03  0:35       ` Max Kirillov
2013-12-11 23:36       ` [RFC] HACK: use anchor mark instead of sel tag Max Kirillov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).