mwparserfromhell icon indicating copy to clipboard operation
mwparserfromhell copied to clipboard

closing_tag and closing_wiki_markup should default to None when identical to opening tag

Open Rua opened this issue 9 years ago • 3 comments

Currently, these two values are automatically reassigned whenever the opening tags are assigned. Assigning a value to wiki_markup will override closing_tag, and likewise for regular tags. This is bad behaviour, it leads to unexpected surprises because you think you are changing one value while you are really changing two. You also have to work yourself into a twist if you want to do processing and updates on both values individually:

oldvalue = node.closing_tag
node.tag = do_stuff(node.tag)
node.closing_tag = do_stuff(oldvalue)

What I think really should happen is that the value "None" simply means "take the value from the opening tag".

Rua avatar Oct 31 '15 21:10 Rua

Rua, can you please clarify what you mean when you say "closing_tag should default to none when identical to opening tag"? I take that to mean that closing tag should be none if it encounters something like <small><small>really small text</small></small> which has two identical opening tags side by side, but I sense you are trying to mean something else and I'm just not getting it. Thanks.

Technical-13 avatar Oct 31 '15 21:10 Technical-13

I'm talking about the properties on the Tag objects themselves. Under the current behaviour, anything that is assigned to the .tag or .wiki_markup properties on a Tag object will overwrite .closing_tag and .closing_wiki_markup with that value as well. My suggestion is that instead of this, the .closing_tag and .closing_wiki_markup values should remain as None until explicitly assigned (by the user or the parser). The code within the Tag class should be changed so that it treats None for these values as indicating that the opening and closing tags are identical.

Rua avatar Oct 31 '15 21:10 Rua

I get the underlying issue, but when do you want to update the opening tag but not the closing one?

earwig avatar Nov 01 '15 09:11 earwig