Page MenuHomePhabricator
Create Task
Maniphest T237241

Implement basic book referencing
Closed, ResolvedPublic8 Estimated Story Points
Actions

Description

I suggest to split the task of implementing the new extends="…" attribute into smaller technical tickets, starting with this one here:

  • In the text, a <ref> with the new attribute is rendered with a sub-number like "1.3".
  • In the <references /> section, the reference is rendered with indentation and sub-numbering.
  • Include parser tests if applicable.
  • Don't deal with any edge cases, where possible. STATUS: accidentally dealt with almost every edge case.
  • Don't touch VisualEditor use cases.

Depends on: T236257

This is the recent proof-of-concept, which visually accomplishes the task here: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Cite/+/530399/

Details

Related Changes in Gerrit:
Show related patches Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedNoneT237241 Implement basic book referencing
 ResolvedNoneT236257 Make PHP code accept new attribute for refinements

Event Timeline

thiemowmde created this task.Nov 4 2019, 11:16 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 4 2019, 11:16 AM
thiemowmde added a subtask: T236257: Make PHP code accept new attribute for refinements.Nov 4 2019, 11:16 AM
awight closed subtask T236257: Make PHP code accept new attribute for refinements as Resolved.Nov 6 2019, 12:48 PM
awight added a project: WMDE-QWERTY-Sprint-2019-11-06.
awight added a comment.Nov 6 2019, 12:55 PM

We're discussing rewriting this ticket without the proposed, intermediate implementation. We would go straight to indented and sub-numbered references.

awight updated the task description. (Show Details)Nov 6 2019, 12:59 PM
awight set the point value for this task to 5.
awight moved this task from Incoming to Tickets in sprint on the Sub-referencing (product board) board.Nov 12 2019, 12:20 PM
awight renamed this task from Implement baseline and/or fallback for non-indented sub-references to Implement basic book referencing.Nov 12 2019, 12:55 PM

Indentation could be split out into its own patches, if that makes the implementation easier.

gerritbot added a comment.Nov 13 2019, 12:50 PM

Change 550672 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/Cite@master] [WIP] Extract numbering functions to separate classes

https://gerrit.wikimedia.org/r/550672

gerritbot added a project: Patch-For-Review.Nov 13 2019, 12:50 PM
gerritbot added a comment.Nov 18 2019, 12:48 PM

Change 551525 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Remove dead code from Cite::referencesFormatEntry()

https://gerrit.wikimedia.org/r/551525

gerritbot added a comment.Nov 18 2019, 1:19 PM

Change 551530 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Avoid intermediate array when rendering a <references> list

https://gerrit.wikimedia.org/r/551530

gerritbot added a comment.Nov 18 2019, 1:43 PM

Change 551536 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Fix indention and add comments to referencesFormatEntry()

https://gerrit.wikimedia.org/r/551536

gerritbot added a comment.Nov 18 2019, 2:01 PM

Change 551539 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Collapse duplicate code in Cite::referencesFormatEntry()

https://gerrit.wikimedia.org/r/551539

gerritbot added a comment.Nov 18 2019, 3:20 PM

Change 551554 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] [WIP] Render nested references

https://gerrit.wikimedia.org/r/551554

gerritbot added a comment.Nov 18 2019, 3:35 PM

Change 551558 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Reduce duplicate code in Cite::stack()

https://gerrit.wikimedia.org/r/551558

gerritbot added a comment.Nov 19 2019, 8:08 AM

Change 551525 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Remove dead code from Cite::referencesFormatEntry()

https://gerrit.wikimedia.org/r/551525

thiemowmde mentioned this in rECIT508f7a05646c: Remove dead code from Cite::referencesFormatEntry().Nov 19 2019, 8:10 AM
gerritbot added a comment.Nov 19 2019, 8:12 AM

Change 551558 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Reduce duplicate code in Cite::stack()

https://gerrit.wikimedia.org/r/551558

thiemowmde mentioned this in rECIT19eefa5d71f6: Reduce duplicate code in Cite::stack().Nov 19 2019, 8:14 AM
gerritbot added a comment.Nov 19 2019, 8:58 AM

Change 551530 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Avoid intermediate array when rendering a <references> list

https://gerrit.wikimedia.org/r/551530

gerritbot added a comment.Nov 19 2019, 8:58 AM

Change 551536 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Fix indention and add comments to referencesFormatEntry()

https://gerrit.wikimedia.org/r/551536

thiemowmde mentioned this in rECITe1519911d887: Avoid intermediate array when rendering a <references> list.Nov 19 2019, 8:59 AM
thiemowmde mentioned this in rECIT97ba23aae917: Fix indention and add comments to referencesFormatEntry().
gerritbot added a comment.Nov 19 2019, 9:00 AM

Change 551539 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Collapse duplicate code in Cite::referencesFormatEntry()

https://gerrit.wikimedia.org/r/551539

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.8; 2019-11-26).Nov 19 2019, 9:00 AM
thiemowmde mentioned this in rECITc4c6eaa8df3d: Collapse duplicate code in Cite::referencesFormatEntry().Nov 19 2019, 9:00 AM
thiemowmde added a project: WMDE-QWERTY-Sprint-2019-11-20.Nov 20 2019, 12:57 PM
awight moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2019-11-06 board.Nov 20 2019, 1:05 PM
awight updated the task description. (Show Details)Nov 20 2019, 1:07 PM
awight changed the point value for this task from 5 to 7.
gerritbot added a comment.Nov 20 2019, 2:48 PM

Change 552073 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Block de-facto empty <ref> as if it's empty

https://gerrit.wikimedia.org/r/552073

gerritbot added a comment.Nov 20 2019, 3:49 PM

Change 552073 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Block de-facto empty <ref> as if it's empty

https://gerrit.wikimedia.org/r/552073

thiemowmde mentioned this in rECITb10dd4ec27e9: Block de-facto empty <ref> as if it's empty.Nov 20 2019, 3:50 PM
Izno subscribed.Nov 20 2019, 7:38 PM
In T237241#5678600, @gerritbot wrote:

Change 552073 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Block de-facto empty <ref> as if it's empty

https://gerrit.wikimedia.org/r/552073

  1. That summary is rather obtuse. Please make it more clear the intent.
  2. You should probably split this into its own task because you are changing rendering behavior here for wikitext that already exists. It's not appropriate to do it as part of this ticket in that regard.
  3. Reminds me of T157418: RFC: Make some aspects of Tidy's whitespace stripping behavior part of wikitext parsing "spec". Maybe one of the sorts on that ticket might have an opinion.
Izno added a comment.Nov 20 2019, 8:06 PM

And probably a 4. The error message displayed is insufficient. Someone looking through the wikitext is going to look for an actually-empty item, not one that is empty save for whitespace.

awight moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2019-11-20 board.Nov 21 2019, 9:41 AM
thiemowmde added a comment.Nov 21 2019, 1:41 PM

Thanks a lot for taking care! I really hope we did not forgot something, but we are all humans. Let's see:

  1. Are you talking about the first line in the commit message? Yes, that could have been more clear. Sorry. For reference: The intent was that <ref> with no content behave the same as <ref> with no visible content.
  2. Yes, that would have been better. However, we consider this not really a change in behavior, but a bugfix. Before, is was possible to have <ref> with no visible content, but no way to find them.
  3. I'm not sure what you are referring to. T157418 appears to be an entirely different, way larger can of worms.
  4. I checked, and it seems the two relevant error messages don't talk about the <ref> being "empty", but "must have content" and "no text". Both do sound good to me. Did I missed something? Which error message do you think is a problem?
Izno added a comment.Nov 21 2019, 2:38 PM
In T237241#5681435, @thiemowmde wrote:
  1. Yes, that would have been better. However, we consider this not really a change in behavior, but a bugfix. Before, is was possible to have <ref> with no visible content, but no way to find them.

It's not a bug fix if no-one asked for it (decent rule of thumb for Wikimedians). If you're changing how stuff is rendered in wikitext especially. See also below comment.

  1. I'm not sure what you are referring to. T157418 appears to be an entirely different, way larger can of worms.

And it's already done. The reason I'm mentioning the task is to ask whether we should apply the same notion of trimming to random XML-like wikitext tags ad hoc. The resolution in that task for example was that wikitext HTML tags do not trim whitespace (according to the HTML 5 construction rules), and only some wikitext does have whitespace trimmed. Do we want an ad hoc change here for all reference tags to trim whitespace? Start getting into questions of whether other tags should trim whitespace. Is a wikitext XML-like tag more like HTML 5 (and should it follow the HTML 5 rules) or is it more like wikitext and should it be allowed to strip whitespace? Not strip whitespace? All such tags? Ad hoc?

  1. Are you talking about the first line in the commit message? Yes, that could have been more clear. Sorry. For reference: The intent was that <ref> with no content behave the same as <ref> with no visible content.
  2. I checked, and it seems the two relevant error messages don't talk about the <ref> being "empty", but "must have content" and "no text". Both do sound good to me. Did I missed something? Which error message do you think is a problem?

No visible content is still content and needs to indicated as such. With a warning that says "no content" or even "no text" (why are those different warnings?), the user will look for ></ref> rather than something like >\w</ref> (pseudo-regex) in the wikitext.

I would expect the commit summary to be "treat whitespace-only ref tags like no-content ref tags" or "trim whitespace in ref tags", since that's what you're doing.

As I said,

It's not appropriate to do it as part of this ticket.

gerritbot added a comment.Nov 21 2019, 4:25 PM

Change 552296 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Merge two more code paths in Cite::referencesFormatEntry()

https://gerrit.wikimedia.org/r/552296

gerritbot added a comment.Nov 22 2019, 2:22 PM

Change 552296 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Merge two more code paths in Cite::referencesFormatEntry()

https://gerrit.wikimedia.org/r/552296

awight mentioned this in rECITa823fa23d90f: Merge two more code paths in Cite::referencesFormatEntry().Nov 22 2019, 2:22 PM
gerritbot added a comment.Nov 22 2019, 5:33 PM

Change 552546 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/Cite@master] Extract stack and state to a new class

https://gerrit.wikimedia.org/r/552546

gerritbot added a comment.Nov 25 2019, 1:38 PM

Change 552546 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Extract stack and state to a new class

https://gerrit.wikimedia.org/r/552546

awight mentioned this in rECIT8453e3ecd740: Extract stack and state to a new class.Nov 25 2019, 1:39 PM
thiemowmde changed the point value for this task from 7 to 8.Nov 26 2019, 12:33 PM
gerritbot added a comment.Nov 26 2019, 1:06 PM

Change 550672 abandoned by Awight:
Extract numbering functions to separate classes

Reason:
Looking at Icd933fc98, I don't think these services are relevant to our work at the moment. Abandoning as out of scope.

https://gerrit.wikimedia.org/r/550672

gerritbot added a comment.Nov 26 2019, 1:50 PM

Change 553111 had a related patch set uploaded (by Awight; owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] [WIP] Render nested references (fork)

https://gerrit.wikimedia.org/r/553111

gerritbot added a comment.Nov 29 2019, 3:19 PM

Change 553111 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Render nested references

https://gerrit.wikimedia.org/r/553111

awight mentioned this in rECIT13598ba11e91: Render nested references.Nov 29 2019, 3:21 PM
ReleaseTaggerBot edited projects, added MW-1.35-notes (1.35.0-wmf.10; 2019-12-10); removed MW-1.35-notes (1.35.0-wmf.8; 2019-11-26).Nov 29 2019, 4:00 PM
gerritbot added a comment.Nov 29 2019, 4:30 PM

Change 553759 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Don't leave unclosed <li> behind

https://gerrit.wikimedia.org/r/553759

gerritbot added a comment.Nov 29 2019, 5:57 PM

Change 551554 abandoned by Awight:
[WIP] Render nested references

Reason:
Merged the alternate fork.

https://gerrit.wikimedia.org/r/551554

gerritbot added a comment.Dec 1 2019, 10:36 AM

Change 553759 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Don't leave unclosed <li> behind

https://gerrit.wikimedia.org/r/553759

awight mentioned this in rECIT3cf1e99cc282: Don't leave unclosed <li> behind.Dec 1 2019, 10:37 AM
Maintenance_bot removed a project: Patch-For-Review.Dec 1 2019, 11:10 AM
awight added a comment.Dec 9 2019, 8:44 AM

We've hit a lot of numbering issues already, I'll just do this here rather than create a new task. On current master, we have:

<ref name="a">First</ref>
<ref extends="a">Second</ref>
<ref extends="a">Third</ref>
<ref name="d">Fourth</ref>
<hr />
<references />

Rendering as,

image.png (161×260 px, 5 KB)

gerritbot added a comment.Dec 9 2019, 8:53 AM

Change 555902 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/Cite@master] Numbering bug: Parser test which should fail

https://gerrit.wikimedia.org/r/555902

gerritbot added a project: Patch-For-Review.Dec 9 2019, 8:53 AM
awight added a comment.Dec 9 2019, 8:59 AM
In T237241#5722632, @awight wrote:

We've hit a lot of numbering issues already, I'll just do this here rather than create a new task. On current master, we have:

Bisecting shows I broke this in rECIT3f276388bff

gerritbot added a comment.Dec 9 2019, 9:21 AM

Change 555906 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/Cite@master] Fix footnote mark after extends numbering glitch

https://gerrit.wikimedia.org/r/555906

awight updated the task description. (Show Details)Dec 9 2019, 9:28 AM
awight moved this task from Doing to Demo on the WMDE-QWERTY-Sprint-2019-11-20 board.
gerritbot added a comment.Dec 9 2019, 10:06 AM

Change 555902 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Numbering bug: Parser test which should fail

https://gerrit.wikimedia.org/r/555902

awight mentioned this in rECITd8433101a7d0: Numbering bug: Parser test which should fail.Dec 9 2019, 10:06 AM
gerritbot added a comment.Dec 9 2019, 12:20 PM

Change 555906 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Fix footnote mark after extends numbering glitch

https://gerrit.wikimedia.org/r/555906

awight mentioned this in rECITf51060eaf418: Fix footnote mark after extends numbering glitch.Dec 9 2019, 12:21 PM
Maintenance_bot removed a project: Patch-For-Review.Dec 9 2019, 1:10 PM
Tobi_WMDE_SW added a project: WMDE-QWERTY-Sprint-2019-12-11.Dec 11 2019, 9:13 AM
Tobi_WMDE_SW moved this task from Sprint Backlog to Demo on the WMDE-QWERTY-Sprint-2019-12-11 board.Dec 11 2019, 9:13 AM
Izno moved this task from Unsorted backlog to Enhancement backlog on the Cite board.Dec 20 2019, 1:37 PM
WMDE-Fisch closed this task as Resolved.Jan 7 2020, 12:59 PM
WMDE-Fisch moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2019-12-11 board.

This ticket was meant as an umbrella for a first version of the feature. I consider it done. Any other things we want to work on in this context should go into their own tickets.

thiemowmde mentioned this in T117037: Named ref are considered different when the content is a whitespace for refs 2 to n.Jan 24 2020, 1:24 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits