jenkins-bot has submitted this change and it was merged.
Change subject: [BREAK][FIX] Redesign of InteractiveReplace ......................................................................
[BREAK][FIX] Redesign of InteractiveReplace
It adds the Choice class which can be (along with ChoiceExceptions) in a separate list to have their own handling and allow to add the same kind of choice multiple times.
The 'y' shortcut has been removed as it is odd because the user doesn't get the same shortcut presented for all the choices. It also doesn't simplify anything as there is also a default option (which might be different). Apart from that it was buggy and it assigned always the last allowed replace option instead of the first which meant that the replace option using 'y' wasn't actually available (as it has been mapped the last one which also has the ordinary shortcut).
Change-Id: I56b32a0f69147fb46772b8a1dffdaef5b19931de --- M pywikibot/bot.py M scripts/disambredir.py M tests/disambredir_tests.py 3 files changed, 127 insertions(+), 69 deletions(-)
Approvals: John Vandenberg: Looks good to me, approved jenkins-bot: Verified
diff --git a/pywikibot/bot.py b/pywikibot/bot.py index 5c516d7..9f0e686 100644 --- a/pywikibot/bot.py +++ b/pywikibot/bot.py @@ -701,6 +701,61 @@ force=force)
+class Choice(StandardOption): + + """A simple choice consisting of a option, shortcut and handler.""" + + def __init__(self, option, shortcut, replacer): + """Constructor.""" + super(Choice, self).__init__(option, shortcut) + self._replacer = replacer + + @property + def replacer(self): + """The replacer.""" + return self._replacer + + def handle(self): + """Handle this choice. Must be implemented.""" + raise NotImplementedError() + + +class StaticChoice(Choice): + + """A static choice which just returns the given value.""" + + def __init__(self, option, shortcut, result): + """Create instance with replacer set to None.""" + super(StaticChoice, self).__init__(option, shortcut, None) + self._result = result + + def handle(self): + """Return the predefined value.""" + return self._result + + +class LinkChoice(Choice): + + """A choice returning a mix of the link new and current link.""" + + def __init__(self, option, shortcut, replacer, section): + """Constructor.""" + super(LinkChoice, self).__init__(option, shortcut, replacer) + self._section = section + + def handle(self): + """Handle by either applying the new section or label.""" + if self._section: + kwargs = {'section': self.replacer._new.section, + 'label': self.replacer.current_link.anchor} + else: + kwargs = {'section': self.replacer.current_link.section, + 'label': self.replacer._new.anchor} + return pywikibot.Link.create_separated( + self.replacer._new.canonical_title(), self.replacer._new.site, + **kwargs) + + class InteractiveReplace(object):
""" @@ -719,12 +774,12 @@ it is greater 0 it shows that many characters before and after the link in question.
- Subclasses may overwrite build_choices and handle_answer to add custom made - answers. + Additional choices can be defined using the 'additional_choices' and will be + amended to the choices defined by this class. This list is mutable and the + Choice instance returned and created by this class are too. """
- def __init__(self, old_link, new_link, default=None, automatic_quit=True, - yes_shortcut=True): + def __init__(self, old_link, new_link, default=None, automatic_quit=True): """ Constructor.
@@ -740,9 +795,6 @@ @param automatic_quit: Add an option to quit and raise a QuitKeyboardException. @type automatic_quit: bool - @param yes_shortcut: Make the first replacement option accessible via - 'y' shortcut (does not apply to unlink). - @type yes_shortcut: bool """ if isinstance(old_link, pywikibot.Page): self._old = old_link._link @@ -754,7 +806,7 @@ self._new = new_link self._default = default self._quit = automatic_quit - self._yes = yes_shortcut + self._current_match = None self.context = 30 self.allow_skip_link = True self.allow_unlink = True @@ -762,64 +814,44 @@ self.allow_replace_section = False self.allow_replace_label = False self.allow_replace_all = False + # Use list to preserve order + self._own_choices = [ + ('skip_link', StaticChoice('Do not change', 'n', None)), + ('unlink', StaticChoice('Unlink', 'u', False)), + ('replace', StaticChoice('Change link target', 't', + self._new.canonical_title())), + ('replace_section', LinkChoice('Change link target and section', + 's', self, True)), + ('replace_label', LinkChoice('Change link target and label', + 'l', self, False)), + ('replace_all', StaticChoice('Change complete link', 'c', + self._new)), + ]
- def build_choices(self): - """ - Return the choices and what the shortcut 'y' actually means. + self.additional_choices = []
- The shortcut alias for 'y' may be either what yes_shortcut is in the - constructor negated or the actual shortcut used. So if it didn't use 'y' - at all it's a boolean (True if there are replacements and it was - disabled, False if there are no replacements and it is enabled) and - otherwise it's one character. - """ - choices = [] - if self.allow_skip_link: - choices += [('Do not change', 'n')] - if self.allow_unlink: - choices += [('Unlink', 'u')] - yes_used = not self._yes - if self.allow_replace: - choices += [('Change link target', 't' if yes_used else 'y')] - yes_used = 't' - if self.allow_replace_section: - choices += [('Change link target and section', 's' if yes_used else 'y')] - yes_used = 's' - if self.allow_replace_label: - choices += [('Change link target and label', 'l' if yes_used else 'y')] - yes_used = 'l' - if self.allow_replace_all: - choices += [('Change complete link', 'c' if yes_used else 'y')] - yes_used = 'c' - # 'y' was disabled in the constructor so return False as it was actually - # not used - if yes_used is True: - yes_used = False - return choices, yes_used - - def handle_answer(self, choice, link): + def handle_answer(self, choice): """Return the result for replace_links.""" - if choice == 'n': - return None - elif choice == 'u': - return False - elif choice == 't': - return self._new.canonical_title() - elif choice == 's': - return pywikibot.Link.create_separated( - self._new.canonical_title(), self._new.site, - section=self._new.section, label=link.anchor) - elif choice == 'l': - return pywikibot.Link.create_separated( - self._new.canonical_title(), self._new.site, - section=link.section, label=self._new.anchor) + for c in self.choices: + if c.shortcut == choice: + return c.handle() else: - assert choice == 'c', 'Invalid choice {0}'.format(choice) - return self._new + raise ValueError('Invalid choice "{0}"'.format(choice)) + + @property + def choices(self): + """Return the tuple of choices.""" + choices = [] + for name, choice in self._own_choices: + if getattr(self, 'allow_' + name): + choices += [choice] + choices += self.additional_choices + return tuple(choices)
def __call__(self, link, text, groups, rng): """Ask user how the selected link should be replaced.""" if self._old == link: + self._current_match = (link, text, groups, rng) if self.context > 0: # at the beginning of the link, start red color. # at the end of the link, reset the color to default @@ -831,23 +863,46 @@ else: question = ('Should the link \03{{lightred}}{1}\03{{default}} ' 'target to \03{{lightpurple}}{0}\03{{default}}?') - choices, yes_alias = self.build_choices() - if yes_alias and self._default == yes_alias: - default = 'y' - else: - default = self._default
choice = pywikibot.input_choice( question.format(self._new.canonical_title(), self._old.canonical_title()), - choices, default=default, automatic_quit=self._quit) - if yes_alias and choice == 'y': - choice = yes_alias + self.choices, default=self._default, automatic_quit=self._quit)
- return self.handle_answer(choice, link) + answer = self.handle_answer(choice, link) + self._current_match = None + return answer else: return None
+ @property + def current_link(self): + """Get the current link when it's handling one currently.""" + if self._current_match is None: + raise ValueError('No current link') + return self._current_match[0] + + @property + def current_text(self): + """Get the current text when it's handling one currently.""" + if self._current_match is None: + raise ValueError('No current text') + return self._current_match[1] + + @property + def current_groups(self): + """Get the current groups when it's handling one currently.""" + if self._current_match is None: + raise ValueError('No current groups') + return self._current_match[2] + + @property + def current_range(self): + """Get the current range when it's handling one currently.""" + if self._current_match is None: + raise ValueError('No current range') + return self._current_match[3] +
# Command line parsing and help def calledModuleName(): diff --git a/scripts/disambredir.py b/scripts/disambredir.py index 4673238..46d6afe 100755 --- a/scripts/disambredir.py +++ b/scripts/disambredir.py @@ -46,7 +46,7 @@
def _create_callback(self, old, new): replace_callback = InteractiveReplace( - old, new, default='n', automatic_quit=False, yes_shortcut=True) + old, new, default='n', automatic_quit=False) replace_callback.allow_replace_label = True return replace_callback
diff --git a/tests/disambredir_tests.py b/tests/disambredir_tests.py index 5e260c7..6598a5d 100644 --- a/tests/disambredir_tests.py +++ b/tests/disambredir_tests.py @@ -49,7 +49,10 @@ def _patched_callback(link, text, groups, rng): """Return the result from handle_answer.""" if callback._old == link: - return callback.handle_answer(choice, link) + callback._current_match = (link, text, groups, rng) + answer = callback.handle_answer(choice) + callback._current_match = None + return answer else: return None