Ticket #2294 (closed defect: fixed)

Opened 9 years ago

Last modified 8 years ago

mcview: incorrect starting offset for 'search again'

Reported by: egmont Owned by: slavazanko
Priority: minor Milestone: 4.8.0-pre1
Component: mcview Version: 4.7.3
Keywords: Cc:
Blocked By: Blocking:
Branch state: no branch Votes for changeset: commited-master commited-stable

Description

When you press F7 in mcview to repeat the previous search, it starts again from the file offset that is 3 bytes after the beginning of the current match. This introduces weird and clearly buggy symptoms. Couple of examples:

If the file contains

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

and you search for a single x character, and you keep on pressing F7, every third character gets highlighted.

If, in the same file, you search for xxxxxxxxxxxxx (many, but way fewer x characters than in the file), the matching window keeps on moving by 3 characters.

If the file contains

xyxyxyxyxyxyxyxyxyxyxyxy

and you search for xy, you get every second match.

If the file is a shell script with comments, such at this:

#!/bin/bash
# 
# copyright stuff
#
# blah

and you search for # then the ones after the empty comment lines are skipped.

Advancing by 3 bytes is totally arbitrary and leads to buggy behavior. Either you should advance by 1 byte, or advance to the end of the previous match (these would behave differently, IMHO both of them make sense).

Attachments

mc-4.7.0.7-search-start-offset.patch (552 bytes) - added by egmont 9 years ago.
fix with minor side effects (see comment)

Change History

comment:1 Changed 9 years ago by andrew_b

  • Component changed from mc-core to mcview

Changed 9 years ago by egmont

fix with minor side effects (see comment)

comment:2 Changed 9 years ago by egmont

Attached patch fixes the problem, while resurrects ticket #265.

Until a permanent solution is created by someone, I recommend applying this patch, marking this bug as fixed and reopening #265. I think it's better to have duplicate matches in a rare use case rather than missing some matches in a more common use case.

comment:3 Changed 8 years ago by slavazanko

  • Owner set to slavazanko
  • Status changed from new to accepted
  • severity changed from no branch to on review
  • Blocking 265 added
  • Milestone changed from 4.7 to 4.8

Created branch 2294_viewer_search_offset

Initial changeset:899bcae078f2dfc82182b05268c4c691a467220b

Review, please.

I think it's better to have duplicate matches in a rare use case rather than missing some matches in a more common use case.

You right, after resolve this issue #265 will be reopened.

comment:4 Changed 8 years ago by slavazanko

  • Votes for changeset set to slavazanko

comment:5 Changed 8 years ago by angel_il

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

comment:6 Changed 8 years ago by slavazanko

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

Merge changeset:becd744bb664ea0b7c6ffeef99113ea851cdc695

For getting list of commits type:

git log --pretty=oneline 906f688..3ad5f51

comment:8 Changed 8 years ago by slavazanko

  • Keywords stable-candidate added

comment:9 Changed 8 years ago by slavazanko

  • Milestone changed from 4.8 to 4.8.0-pre1

comment:10 Changed 8 years ago by slavazanko

  • Status changed from testing to closed
  • Keywords stable-candidate removed
  • Votes for changeset changed from commited-master to commited-master commited-stable
  • Branch state set to no branch

Cherry-picked in stable:

git log --pretty=oneline 52047442..a1cc8f37e
Note: See TracTickets for help on using tickets.