Slightly improve handling of Scribunto/non-wikitext content models #857
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Amorymeltzer
added
bug
Module: speedy
Module: morebits
Amorymeltzer
changed the title
Amorymeltzer
changed the title
Amorymeltzer
force-pushed
the
watchmodules
branch
from
a0e6c15
to
fecc728
Compare
|
Another way of doing the first commit would be to rely on |
…pNonRedirectCreator is provided PR wikimedia-gadgets#710 added the ability for `fnLookupCreationSuccess` to find the creator of the first non-redirect revision via `fnLookupNonRedirectCreator` (see also wikimedia-gadgets#805) by adding `content` to the `rvprop`s list as well as `rvsection=0`. That doesn't work for non-wikitext content models, who will returned a `rvnosuchsection` error in such cases. Removing `content` would solve the error, as would `rvsection=0`, but the actual redirect check (`/^\s*#redirect/i`) doesn't make sense in those scenarios (e.g. Scribunto, JS, CSS), so there's no really point in even checking. Instead, only attempt to use `content` and `rvsection` if `lookupNonRedirectCreator` is set. Currently, only AfD and PROD do so, which aligns with how it should be limited. We could probably just use `wgPageContentModel`, but in theory the ability to query pages other than the current one, even if the two have different content models, seems useful. In practice, only `twinklexfd` attempts sch a thing, only for TfD merge nominations (`involvedpages`), and assumes that the target is in the same namespace as the current page, so Scribunto will match with Scribunto.
Uses params.scribunto to check if we're dealing with a /doc subpage or not (easier and more portable than regex testing, etc.), and simply call the `watch` api. Closes wikimedia-gadgets#572. Also removes some collateral with undesired `pageobj.setCreateOption('recreate')` for non-module taggings.
Amorymeltzer
force-pushed
the
watchmodules
branch
from
fecc728
to
133d227
Compare
|
Now works as I mentioned in #857 (comment) |
Amorymeltzer
added a commit
to Amorymeltzer/twinkle
that referenced
this pull request
Mar 22, 2020
…placements Better and more complete way of doing 133d227 (wikimedia-gadgets#857), for both CfD and TfD, as well as some tidying for CfD/S. Only replaces the current namepsace, which is basically what was being done anyway, but this way it's obvious when we're pigeonholing a page into an incorrect namespace. For CfD/S, the behavior is basically inverted: instead of removing the namespace then adding it for just the discussion page, this removes it just for the tagging.
Amorymeltzer
added a commit
to Amorymeltzer/twinkle
that referenced
this pull request
Mar 22, 2020
…placements Better and more complete way of doing 133d227 (wikimedia-gadgets#857), for both CfD and TfD, as well as some tidying for CfD/S. Only replaces the current namepsace, which is basically what was being done anyway, but this way it's obvious when we're pigeonholing a page into an incorrect namespace. For CfD/S, the behavior is basically inverted: instead of removing the namespace then adding it for just the discussion page, this removes it just for the tagging.
Amorymeltzer
added a commit
to Amorymeltzer/twinkle
that referenced
this pull request
Apr 19, 2020
…placements Better and more complete way of doing 133d227 (wikimedia-gadgets#857), for both CfD and TfD, as well as some tidying for CfD/S. Only replaces the current namepsace, which is basically what was being done anyway, but this way it's obvious when we're pigeonholing a page into an incorrect namespace. For CfD/S, the behavior is basically inverted: instead of removing the namespace then adding it for just the discussion page, this removes it just for the tagging.
Amorymeltzer
added a commit
to Amorymeltzer/twinkle
that referenced
this pull request
Apr 25, 2020
…placements Better and more complete way of doing 133d227 (wikimedia-gadgets#857), for both CfD and TfD, as well as some tidying for CfD/S. Only replaces the current namepsace, which is basically what was being done anyway, but this way it's obvious when we're pigeonholing a page into an incorrect namespace. For CfD/S, the behavior is basically inverted: instead of removing the namespace then adding it for just the discussion page, this removes it just for the tagging.
wiki-ST47
pushed a commit
to wiki-ST47/twinkle
that referenced
this pull request
Sep 2, 2020
…placements Better and more complete way of doing 133d227 (wikimedia-gadgets#857), for both CfD and TfD, as well as some tidying for CfD/S. Only replaces the current namepsace, which is basically what was being done anyway, but this way it's obvious when we're pigeonholing a page into an incorrect namespace. For CfD/S, the behavior is basically inverted: instead of removing the namespace then adding it for just the discussion page, this removes it just for the tagging.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1st commit: morebits: lookupCreation won't query content and section unless lookupNonRedirectCreator is provided
PR #710 added the ability for
fnLookupCreationSuccessto find the creator of the first non-redirect revision viafnLookupNonRedirectCreator(see also #805) by addingcontentto thervprops list as well asrvsection=0. That doesn't work for non-wikitext content models, who will returned arvnosuchsectionerror in such cases. Removingcontentwould solve the error, as wouldrvsection=0, but the actual redirect check (/^\s*#redirect/i) doesn't make sense in those scenarios (e.g. Scribunto, JS, CSS), so there's no really point in even checking.Instead, only attempt to use
contentandrvsectioniflookupNonRedirectCreatoris set. Currently, only AfD and PROD do so, which aligns with how it should be limited.We could probably just use
wgPageContentModel, but in theory the ability to query pages other than the current one, even if the two have different content models, seems useful. In practice, onlytwinklexfdattempts sch a thing, only for TfD merge nominations (involvedpages), and assumes that the target is in the same namespace as the current page, so Scribunto will match with Scribunto.2nd commit: xfd/speedy: Watch tagged modules
Uses params.scribunto to check if we're dealing with a /doc subpage or not (easier and more portable than regex testing, etc.), and simply call the
watchapi. Also removes some collateral with undesiredpageobj.setCreateOption('recreate')for non-module taggings. Closes #572; Sorry if you were actively looking at this @DannyS712 and I scooped you, I got thinking after the above.3rd commit: xfd: Fix tfm preview when namespace is included