Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editor.rereplace() should not replace if data isn't going to be changed #272

Open
alankilborn opened this issue Jan 9, 2023 · 4 comments

Comments

@alankilborn
Copy link

Consider this script:

def add_1(m):
    z = int(m.group(1))
    return 'Y' + str(z + 1) if z < 100 else m.group(0)

# replace X followed by numbers by an incremented number
# e.g.   X999
# e.g.   X56 X39
# e.g.   X999
# e.g.   X56 X39
# e.g.   X999
# e.g.   X56 X39
#          becomes
#        Y57 Y40 Y1000

editor.rereplace('X([0-9]+)', add_1);

Running this script upon its own source file tab results in:

image

Note that lines 7, 9, 11 have the change-history modified marker, even though the text on this lines is the same as it was before the script was run and the replacements made.

Ideally the text that isn't changed wouldn't be replaced and the result after the replacement would look like this:

image

Using PS2.0 and PS3.10.4.

@chcg
Copy link
Collaborator

chcg commented Jan 10, 2023

Unsure if this is a PS topic or one for scintilla.
@alankilborn Why do you think there is no change for X999 where the else case m.group(0) is used for the replacement?
Seems like searching for ABC and replacing with ABC. N++ also shows the change history marker if this is done via GUI.

@alankilborn
Copy link
Author

alankilborn commented Jan 10, 2023

Why do you think there is no change for X999 where the else case m.group(0) is used for the replacement?

Umm, because there is no (real) change to the text content.


Seems like searching for ABC and replacing with ABC.

Yes, also should (ideally be smart enough to) not change the text.


N++ also shows the change history marker if this is done via GUI.

There's another issue forum for fixing N++'s problems. :-)


My real use-case is when I'm trying to use a replace function for text that is a datestamp. I want to replace if a datestamp is older than a certain amount of time, and only then. Since I can't write a regex to say "newer than 15 days ago", I have to do it with a custom replacement function. BUT...a custom replacement function ALWAYS replaces, marking text changed even if content-wise, it hasn't.

Maybe what rereplace() needs is a way to tell its calling code "Don't make this replacement!". Not sure how that would be best achieved? The custom replacement function returns None?

I could achieve my date-based goals by writing my own search-and-replace functions using editor.searchInTarget() and so on... Feels like reinventing the wheel, though.

@alankilborn
Copy link
Author

An interesting somewhat related conversation: https://community.notepad-plus-plus.org/topic/24105

@alankilborn
Copy link
Author

alankilborn commented Feb 9, 2023

I decided to "mock up" how this could work; a script I call RereplaceImproved.py:

# -*- coding: utf-8 -*-
from __future__ import print_function

# references:
#  https://github.com/bruderstein/PythonScript/issues/272 "editor.rereplace() should not replace if data isn't going to be changed"
#  https://github.com/bruderstein/PythonScript/issues/182 "Feature request: editor.rereplace() could return number of replacements made"
#  the code in this script "accomodates" this problem by going to extra lengths to work around it:
#   https://github.com/bruderstein/PythonScript/issues/191 "Crash when using editor.research(regex, lambda m: matches_list.append(m))"
#  https://docs.python.org/3/library/re.html#match-objects

from Npp import *

def rereplace_improved(search_regex, replace_text_or_function, flags=0, start_pos=0, end_pos=0, max_count=0):

    class Match(object):  # note, this is a simplified version of a match class
        def __init__(self, span_list, groups_list, lastindex):
            self.span_list = span_list
            self.groups_list = groups_list
            self.lastindex = lastindex
        def span(self, group_number):
            return self.span_list[group_number]
        def group(self, group_number):
            return self.groups_list[group_number]
        def groups(self):
            return tuple(self.groups_list[1:])  # note: DON'T include group #0 (the entire match)
        def expand(self, text):
            for g in range(len(self.groups_list)):
                text = text.replace('${{{g}}}'.format(g=g), self.group(g))  # replace ${1} with group 1 content, etc.
                text = text.replace('${g}'.format(g=g), self.group(g))  # replace $1 with group 1 content, etc.
                text = text.replace(r'\{g}'.format(g=g), self.group(g))  # replace \1 with group 1 content, etc.
            return text

    replacement_count = 0

    matches = []

    def match_found(m):
        num_groups = len(m.groups())
        span_list = []
        groups_list = []
        for g in range(num_groups + 1):
            span_list.append(m.span(g))
            groups_list.append(m.group(g))
        matches.append(Match(span_list, groups_list, m.lastindex))
    editor.research(search_regex, match_found, flags, start_pos, end_pos, max_count)

    # reverse list so we deal with higher position changes first, so that we don't
    #  have to adjust subsequent positions after changing text like we would if we
    #  started with the lower position matches first
    matches = matches[::-1]

    editor.beginUndoAction()
    for m in matches:
        (group0_start_pos, group0_end_pos) = m.span(0)
        matched_text = editor.getTextRange(group0_start_pos, group0_end_pos)
        new_text = replace_text_or_function(m) if callable(replace_text_or_function) else m.expand(replace_text_or_function)
        if new_text is not None and new_text != matched_text:
            editor.setTargetRange(group0_start_pos, group0_end_pos)
            editor.replaceTarget(new_text)
            replacement_count += 1
    editor.endUndoAction()

    return replacement_count

When used with a replacement function, if that function returns None or text that is the same as the entire search match, the match text will NOT be replaced, and thus not disrupting true change history. This is true also if the replacement is not a function but just simple text -- if that text is the same as what is matched with the search regex, the replacement is not actually made (as it wouldn't change the text anyway).

Note also that the core function of the script implements #182 in that it returns a replacement_count.

Note also that the script implements a Match class; this could have been avoided if not for the bug in #191 that disallows the use of the built-in match stuff. The Match class in the script is NOT as full-featured, however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants