Ticket #4072 (closed defect: fixed)
mcedit anchored regex line matching inconsistencies
| Reported by: | ply | Owned by: | andrew_b |
|---|---|---|---|
| Priority: | minor | Milestone: | 4.8.34 |
| Component: | mcedit | Version: | 4.8.24 |
| Keywords: | Cc: | ||
| Blocked By: | Blocking: | ||
| Branch state: | merged | Votes for changeset: | committed-master |
Description (last modified by andrew_b) (diff)
The editor's F4/Replace function with regex matching shows some broken behavior when the regex begins with the line start anchor, and matches whole lines. Try specifying "^.*" for source and "X\0" for target pattern to try to prefix each line with an "X" to see the following behaviors:
- For consecutive matching lines the replacement only happens on every other line, skipping each candidate that follows a matched line. Maybe matches on consecutive lines are considered overlapping? They shouldn't be.
- When such a regex fully matches the current line, and the cursor is on the first column, the first match is the following line only. Except when the cursor is on the first line, then the first line also matches. I'd expect the current line to match even if the former cases.
Attachments
Change History
comment:3 Changed 9 months 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.34
Branch: 4072_mcedit_replace_bol
Initial changeset:cf686c5fc3f9a7fd593dd3342a5056108a2b6d0d
comment:5 follow-up: ↓ 6 Changed 9 months ago by zaytsev
I'm happy to see new tests, but they fail on Ubuntu and macOS in CI. Probably because of PCRE2? Build logs are attached or available from GitHub.
comment:6 in reply to: ↑ 5 Changed 9 months ago by andrew_b
- Branch state changed from on review to on rework
Replying to zaytsev:
Probably because of PCRE2?
Yes, with PCRE2 replace works unexpected at end of file.
comment:7 Changed 8 months ago by andrew_b
So, now I have tests passed with glib and pcre2 search engines but not with pcre.
The pcre library is at version 8.45. This version of PCRE is now at end of life, and is no longer being actively maintained. Version 8.45 is expected to be the final release of the older PCRE library. The latest release was in 2021-06-22.
Honestly speaking, I don't want to waste my time with pcre. I propose to drop the pcre engine from mc.
comment:8 follow-up: ↓ 9 Changed 8 months ago by zaytsev
Honestly speaking, I don't want to waste my time with pcre. I propose to drop the pcre engine from mc.
I agree. But actually, I wonder why we need direct support for PCRE2 at all. Do you know if there are good reasons for it? I thought that glib provides regex support, so we can just rely on that.
comment:9 in reply to: ↑ 8 Changed 8 months ago by andrew_b
Replying to zaytsev:
Honestly speaking, I don't want to waste my time with pcre. I propose to drop the pcre engine from mc.
I agree. But actually, I wonder why we need direct support for PCRE2 at all. Do you know if there are good reasons for it?
Some historical reasons. GRegex as PCRE wrapper was introduced in GLib-2.14. PCRE was required for MC on OSes with GLib < 2.14. Currently MC requires GLib >= 2.32. Since 2.73.2 (released at 2022-07-08), GRegex is based on PCRE2.
I thought that glib provides regex support, so we can just rely on that.
I agree, we can remove PCRE and PCRE2 from MC and use GLib only.
comment:10 follow-up: ↓ 11 Changed 8 months ago by zaytsev
I agree, we can remove PCRE and PCRE2 from MC and use GLib only.
Sounds great to me. Your explanation makes sense. Please feel free to do so in this ticket, or create another one. Thank you very much!
comment:11 in reply to: ↑ 10 Changed 8 months ago by andrew_b
comment:14 Changed 8 months ago by andrew_b
- Branch state changed from on rework to on review
Rebased to current master.
Branch: 4072_mcedit_replace_bol
Initial changeset:760f09fa85116ee8d570cd52acc587cd8d64282f
comment:15 Changed 8 months ago by zaytsev
Now tests still fail on macOS.
comment:16 Changed 8 months ago by andrew_b
Fixed.
comment:17 Changed 8 months ago by zaytsev
- Votes for changeset set to zaytsev
- Branch state changed from on review to approved
In the first commit, "Ticket 4072" is missing #. Otherwise, it looks good and I'm happy about the tests. Sorry I can't give a more satisfying review.
comment:18 Changed 8 months ago by andrew_b
- Status changed from accepted to testing
- Votes for changeset changed from zaytsev to committed-master
- Resolution set to fixed
- Branch state changed from approved to merged
Merged to master: [be21a83f13cd755ed89de13c7ba953cbfb7679ae].
git log --oneline 4d02bb20a..be21a83f1
The source regex was supposed to be caret-dot-asterisk, but Trac apparently interpreted it...