Ticket #3524 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

Search regex + whole word -> not highlighted

Reported by: egmont Owned by: andrew_b
Priority: minor Milestone: 4.8.15
Component: mc-search Version: master
Keywords: Cc: zaytsev
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

mcview, search for something with "Regular expression" and "Whole words" enabled.

If there's no match, it's properly reported so.

If there's a match, however, the viewport is properly scrolled vertically, but the search result is not highlighted. Plus, you can press 'n' once (or more times if there are multiple matches in the line) and it won't progress to the next match.

Attachments

mc-3524-regex-lookaround.patch (2.0 KB) - added by egmont 4 years ago.
Fix'n'cleanup

Change History

comment:1 Changed 4 years ago by egmont

My instincts tell me that probably in lib/search/normal.c surrounding the regex with that \p{L}\p{N} stuff screws up referring to the matching part.

comment:2 Changed 4 years ago by egmont

It was broken by commit a4673b9 (#2396):

When the search type is not Normal but something else (Regex, Hex or Wildcard), and Whole words is enabled, the code that adds the guards for whole words is _not_ executed (that's bug #3525), yet when fetching the matching part of the regex the code believes it was added there.

Strictly speaking, fixing #3525 would also eliminate this bug. However, the code is unnecessary complicated, since the authors of that patch weren't aware of the awesome regex feature called lookahead and lookbehind.

With these features (actually their negative counterparts) the code can be simplified, and this bug fixed on its own; see the patch. I recommend applying this patch.

By the way, even reverting a4673b9 works for me. I can't reproduce #2396, \b works nicely for me, so I don't know why that was necessary. The fix is 5 years old, who knows what changed since then in glib. Instead of the negative look{ahead,behind} approach I'd be totally fine with bringing back those \b's.

Changed 4 years ago by egmont

Fix'n'cleanup

comment:3 Changed 4 years ago by zaytsev

  • Cc zaytsev added

Thank you for making a proper report! Subscribing myself.

comment:4 Changed 4 years ago by zaytsev

Strictly speaking, fixing #3525 would also eliminate this bug. However, the code is unnecessary complicated, since the authors of that patch weren't aware of the awesome regex feature called lookahead and lookbehind.

You actually meant #2396 and not the bug you filed yourself, right?

comment:5 Changed 4 years ago by egmont

I meant both.

Reverting the change that caused this bug (2396) would obviously fix it.

But instead fixing 3525 only would also fix it I think (intelligently applying the patch [just moving a block of code without changing it] on top of the current codebase), since the wrapping of the regex would be consistent with fetching the 2nd matched segment later on.

But it would I think (haven't tried either) introduce another bug: regexps using backreferences would break since the indices would be shifted.

Long story short: Let's apply both patches, it cleans up the code and makes it more robust.

comment:6 Changed 4 years ago by andrew_b

  • Owner set to andrew_b
  • Status changed from new to accepted
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.15

Branch: 3524_highlight_whole_word
changeset:a0741b0a08b50cc5e1b25e1fa79721a6ff0e568c

comment:7 Changed 4 years ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:8 Changed 4 years ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

Thanks! Applied.
Merged to master: [a0741b0a08b50cc5e1b25e1fa79721a6ff0e568c].

comment:9 Changed 4 years ago by andrew_b

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.