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

Add insertAfterTemplates to Morebits.wikitext.page, respect MOS:ORDER when tagging, update hatnotes #1022

Merged
merged 4 commits into from Jul 14, 2020

Conversation

Amorymeltzer
Copy link
Collaborator

As noted on WT:TW, Twinkle isn't great about MOS:ORDER when tagging things. Accordingly, this:

  • 1st commit adds a function to Morebits.wikitext.page to insert a string after a provided template regex. This is basically copied from the handling in tag. Takes either a string or an array and allows for custom flags.
  • 2nd commit uses that in xfd/protect/prod/csd/tag, with tag getting the extra benefit of finally learning about protection tags ({{pp}}, {{pp-move}}, etc.). A new hidden preference, hatnoteRegex, is added to store the "various hatnotes templates` regex in a unified location.
  • 3rd commit expands thats regex with most hatnotes from Template:Hatnote templates with more than a hundred or so transclusions.

There are some more notes in the individual commits. The main annoyance is that AfD has a comment before the actual template, so having to support that as well is a drag. Maybe the option to specify a flag for the regex for insertAfterTemplates is unnecessary?

Copy link
Member

@siddharthvp siddharthvp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty neat. It would be great if you can include some unit tests too, as you go about testing this. Would make any future changes much easier to verify.

twinkle.js Outdated
@@ -126,6 +126,9 @@ Twinkle.defaultConfig = {
// Hidden preferences
autolevelStaleDays: 3, // Huggle is 3, CBNG is 2
revertMaxRevisions: 50, // intentionally limited
// various hatnote templates, used when tagging
// (csd/xfd/tag/prod/protect) to ensure MOS:ORDER
hatnoteRegex: 'short description|hatnote|main|correct title|dablink|distinguish|for|further|selfref|year dab|similar names|highway detail hatnote|broader|about(?:-distinguish| other people)?|other\\s?(?:hurricane(?: use)?s|people|persons|places|ships|uses(?: of)?)|redirect(?:-(?:distinguish|synonym|multi))?|see\\s?(?:wiktionary|also(?: if exists)?)',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a user preference at all. What we're missing is a way to share data/code between modules. It would be better to define this as Twinkle.hatnoteRegex in twinkle.js file at the bottom under a header called something like:

/**
 * Data and code shared between multiple modules,
 * but not generic enough to be in morebits.
 */

Such a section will also come handy for a lot of other stuff too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly right. I wrote it that way at first, but shied away from it because there simply wasn't anything else there. I thought having it in defaultConfig might make it more obvious that "this is a thing to change," but, yes, it'd be better to just do it normally.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a good place for #969

morebits.js Outdated

// Regex is extra complicated to allow for templates with
// parameters and to handle whitespace properly
return this.text.replace(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return this.text.replace(
return this.text = this.text.replace(

At the least, this should modify this.text. I'm not sure whether it'd better to return the modified text or just this (which would make the method chainable). Presently, the other Morebits.wikitext methods don't return anything so changing them all to return this is what would make sense IMO, so that it can be used like text = wikipage.insertAfterTemplates(...).getText().

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that makes sense, and chaining would be ideal. I can toss in some return this-es to the others, IIRC most of the uses are simple.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused on enwiki, so added here.

morebits.js Outdated
Comment on lines 4023 to 4038
// .length is only a property of strings and arrays so we
// shouldn't need to check type
if (!regex || !regex.length) {
return tag + this.text;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to throw when the input is clearly invalid, so that the client can know they're doing something wrong.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, although I'm mixed on how much of an error/invalid we should actually consider these. I guess it can just be handled on the "client" side, but this seemed nicer. You're probably right about giving them the option of wanting to know.

Would you think both lack of tag and lack of regex` should throw, or just one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with both

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose technically we could check the flags, something like /^[gimsuy]$/

morebits.js Outdated Show resolved Hide resolved
@Amorymeltzer
Copy link
Collaborator Author

It would be great if you can include some unit tests too

I'll get around to #946/#944/#911 soon, hopefully.

@Amorymeltzer Amorymeltzer marked this pull request as ready for review July 9, 2020 10:23
@Amorymeltzer Amorymeltzer added this to the July 2020 update milestone Jul 12, 2020
Toss `return this` at the end of the methods.  Uses of `removeLink` and `commentOutImage` were pretty simple and amenable to chaining; `removeTemplate` and `addToImageComment` are currently unused by Twinkle.
Takes the structure of the regex from the `tag` module, and uses it in `Morebits.wikitext.page.prototype.insertAfterTemplates` to properly find complex templates in the page wikitext, then insert a given string after them.  The regex is identical to what has been used in `tag`:

- The regex for a series of templates is provided as either a string or as an array, from which each item will be pipe-separate.  This is what in `tag` is the CSD, PROD, and "various hatnote templates" section.
- An optional regex flag can be provided; default is `\i`.
- There's an optional `preRegex` for stuff that comes *before* the templates proper (i.e., before `{{`).  This is mainly because enWiki's AfD has an annoying hidden html comment before the `/dated` transclusion.

We could add `postRegex` in the future, although that's likely overengineering
Uses `Morebits.wikitext.page.insertAfterTemplates` with a customized regex string in the hidden preference string `hatnoteRegex`.  `tag` adds on the deletion templates (CSD/PROD/XfD), and finally learns about protection templates.
Barely been updated in a decade; `the` was deleted (https://en.wikipedia.org/wiki/Wikipedia:Templates_for_discussion/Log/2011_January_17#Template:The) in 2011!!!  Includes most templates from https://en.wikipedia.org/w/index.php?title=Template:Hatnote_templates with more than ~100, 150 transclusions.  A few commonish redirects included as well.
@Amorymeltzer Amorymeltzer merged commit 40592a4 into wikimedia-gadgets:master Jul 14, 2020
@Amorymeltzer Amorymeltzer deleted the mosorder branch July 14, 2020 11:05
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Dec 12, 2020
Was part of wikimedia-gadgets#1022 for `insertAfterTemplates`, but wasn't present for `removeLink`, `commentOutImage`, `addToImageComment`, and `removeTemplate`.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Dec 16, 2020
Was part of wikimedia-gadgets#1022 for `insertAfterTemplates`, but wasn't present for `removeLink`, `commentOutImage`, `addToImageComment`, and `removeTemplate`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants