mwparserfromhell icon indicating copy to clipboard operation
mwparserfromhell copied to clipboard

Improve handling of whitespace when adding template parameters

Open Abbe98 opened this issue 8 years ago • 10 comments

Example(warp_status was added):

 |inscriptions     =
 |notes            =
 |other versions   =
|warp_status=
 warped}}

&&

 |longitude        =
 |warped           = ok
|warp_status=
 warped}}

Abbe98 avatar Aug 08 '16 08:08 Abbe98

Where is the code that did this?

lahwaacz avatar Aug 08 '16 11:08 lahwaacz

self.wikitext is a wikicode object, self.page is a pywikibot page object

    def addPropertyToTemplate(self, template, property, value, msg):
        for t in self.wikitext.filter_templates():
             if t.name.matches(template):
                 t.add(property, value)
                 self.page.text = str(self.wikitext)
                 self.page.save(msg)

2016-08-08 13:00 GMT+02:00 Jakub Klinkovský [email protected]:

Where is the code that did this?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/earwig/mwparserfromhell/issues/155#issuecomment-238204789, or mute the thread https://github.com/notifications/unsubscribe-auth/ACgoJ0YHrj1IlGg-4TEVCuDkD0jV4ig-ks5qdwxDgaJpZM4Jez9z .

Abbe98 avatar Aug 08 '16 11:08 Abbe98

What is the type of t? The Template does not have an insert method.

If this is a typo and you meant add instead of insert, what are the values of t, property and value which lead to this behaviour? I'm not going to guess from your first post because one of the explanations is that value contains the wrong whitespace.

lahwaacz avatar Aug 08 '16 12:08 lahwaacz

It was a typo and yes t is a template object. value contains no whitespace at all(t, value, and property both when this occurred and not).

Abbe98 avatar Aug 08 '16 12:08 Abbe98

Thanks, but that's still not a minimal working example. So far I can only guess what the values are and everything seems to work as expected.

lahwaacz avatar Aug 08 '16 12:08 lahwaacz

I will have a hard time creating a minimal working example as it's a inconsistent issue here is the contribution log for reference.

Abbe98 avatar Aug 08 '16 16:08 Abbe98

The bug makes sense based on how Template.add works. At the moment, it only tries to match whitespace before and after the parameter name and value based on the most common existing pattern. It interprets every parameter in your example as having whitespace before its value, so the added parameter gets formatted like that.

A proper fix to that method's "whitespace guessing" ability will need to understand alignment rules and the need for extra whitespace before a parameter even begins in order to handle cases like this. Does (e.g.) AWB have code for that we can base ours off of?

earwig avatar Aug 08 '16 17:08 earwig

I guess a possible hack would be to append the last parameter with a line-break/whitespace?

I'm not familiar with the AWB source code.

Abbe98 avatar Aug 08 '16 19:08 Abbe98

I partially fixed it—the new param does get placed on the correct line—but it's not perfect. I would like to revisit this algorithm in the future at some point.

(Implementation hints: the first and last parameters should be treated differently; try to recognize various spacing conventions by default like the aligned parameter names, whitespace after newline, etc.)

earwig avatar Jun 23 '17 06:06 earwig

@earwig I have found a few more templates where the updated code has trouble with formatting. This is similar to the behavior I described in #185:

  1. Randolph County, Indiana There's an extra | after the name of the infobox. My guess is that's the problem here.
@@ -9,8 +9,7 @@
  |area_land_sq_mi = 452.38 
  |area_water_sq_mi = 0.94
  |area percentage = 0.21% 
- |census yr = 2010
- |pop = 26171
+ |census estimate yr = 2016|pop = 25082<ref name=PopHousingEst>{{cite web|url=https://www.census.gov/programs-surveys/popest.html|title=Population and Housing Unit Estimates |date=August 16, 2017 |accessdate=August 16, 2017|publisher=[[U.S. Census Bureau]]}}</ref>
  |density_km2 =22.31
  |web = http://randolphcounty.us/
 | district = 6th
  1. Catron County, New Mexico Not sure about this one
@@ -10,8 +10,7 @@
 | area_land_sq_mi  = 6924 
 | area_water_sq_mi = 5.5 
 | area percentage = 0.08% 
-| census yr = 2010
-| pop = 3725 
+| census estimate yr = 2016| pop = 3508<ref name=PopHousingEst>{{cite web|url=https://www.census.gov/programs-surveys/popest.html|title=Population and Housing Unit Estimates |date=August 16, 2017 |accessdate=August 16, 2017|publisher=[[U.S. Census Bureau]]}}</ref> 
 | density_sq_mi = 0.5
 | web = www.catroncounty.us
 | district = 2nd
  1. Transylvania County, North Carolina I think this template is formatted correctly, but oddly switches orientation of the separators halfway through.
@@ -10,8 +10,8 @@
  area_land_sq_mi = 379 |
  area_water_sq_mi = 2.0 
 | area percentage = 0.5% 
-| census yr = 2010 
-| pop = 33090 
+|
+ census estimate yr = 2016| pop = 33482<ref name=PopHousingEst>{{cite web|url=https://www.census.gov/programs-surveys/popest.html|title=Population and Housing Unit Estimates |date=August 16, 2017 |accessdate=August 16, 2017|publisher=[[U.S. Census Bureau]]}}</ref> 
 | density_sq_mi = 87 
 | web = www.transylvaniacounty.org 
 | ex image = Brevard, North Carolina - Transylvania Co. Courthouse.JPG
  1. East Baton Rouge Parish, Louisiana The newlines are fine here, but the whitespace around the key/value are off. Everything else has consistent spacing. Maybe because of all the extra properties with no values?
@@ -87,6 +87,9 @@
 | elevation_m             = 
 | population_as_of        = 2015
 | population_footnotes    = 
+| pop_est_as_of= 2016
+| pop_est_footnotes= <ref name=PopHousingEst>{{cite web|url=https://www.census.gov/programs-surveys/popest.html|title=Population and Housing Unit Estimates |date=August 16, 2017 |accessdate=August 16, 2017|publisher=[[U.S. Census Bureau]]}}</ref>
+| population_est= 447037
 | population_total        = 446753
 | population_rank         = LA: [[List of parishes in Louisiana|1st]]
 | population_density_km2  = auto
  1. Nantucket County, Massachusetts Same as above?
@@ -42,6 +42,9 @@
 |area_water_sq_mi         = 57.5
 |elevation_m              = 9
 |elevation_ft             = 30
+|pop_est_as_of= 2016
+|pop_est_footnotes= <ref name=PopHousingEst>{{cite web|url=https://www.census.gov/programs-surveys/popest.html|title=Population and Housing Unit Estimates |date=August 16, 2017 |accessdate=August 16, 2017|publisher=[[U.S. Census Bureau]]}}</ref>
+|population_est= 11008
 |population_total         = 10,172
 |population_as_of         = 2010
 |population_density_km2   = 82.2

betson avatar Aug 16 '17 20:08 betson