5j9 / wikitextparser

A Python library to parse MediaWiki WikiText
GNU General Public License v3.0
289 stars 22 forks source link

Add several missing parser-functions #69

Closed TrueBrain closed 4 years ago

TrueBrain commented 4 years ago

As by https://www.mediawiki.org/wiki/Help:Magic_words, found by parsing https://wiki.openttd.org/

Knowledge of the day: if you give people 15 years to do what-ever on a wiki, they will find every function possible.

codecov[bot] commented 4 years ago

Codecov Report

Merging #69 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #69   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           31        31           
  Lines         4252      4252           
=========================================
  Hits          4252      4252           
Impacted Files Coverage Δ
wikitextparser/_config.py 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5d5057d...7216799. Read the comment docs.

5j9 commented 4 years ago

The thing about these variables is that none of them takes arguments. So, at the time, I thought it wouldn't make much difference if they were detected as templates or parser functions.

Also, https://www.mediawiki.org/wiki/Help:Magic_words differentiates between what it calls "Variables" and "Parser functions":

Variables: these are uppercase words surrounded by double braces, e.g. {{FOO}}. As such, they look a lot like templates. Parser functions: these take parameters and are either of the form {{foo:...}} or {{#foo:...}}, e.g. {{#invoke:}}.

it does not make a clear distinction though IMO.

I'm not %100 sure how I should proceed here, but if you still think it is more consistent this way I'm willing to merge this change.

TrueBrain commented 4 years ago

They kinda made a mess of the spec, didn't they? :D If I understand it correctly:

Variables are always uppercase, but parser-functions can be too (REVISIONID for example). Parser-functions take parameters, but variables can too (NAMESPACE for example).

The only thing that does seem to hold true: parser-functions always have at least one parameter.

So would it be an idea to introduce next to parser-functions a new function called variables, that outputs the variables (which might or might not have parameters), where we put NAMESPACE, PAGENAME, SERVER, etc? And change the parser-functions implementation again that it does not accept anything if it does not have a parameter? I will try today to implement this, just to get a feel for it if it is a sane thing to do :D

5j9 commented 4 years ago

As the distinction is not clear, and the current ParserFunction type provides needed methods, I prefer not to introduce a new type and I think the current idea of making all variables parser functions is good enough for now, until a more specific use-case comes up.

TrueBrain commented 4 years ago

While looking how to implement this, I fully agree; it would only introduce duplicated code for no real reason other than "being correct". Also .. I found PAGEID .. which is both a variable and a parser-function. 🤦

5j9 commented 4 years ago

First of all, sorry that if I'm taking too long to respond. I'm busy these days and hardly find enough the time to focus on this project.

And, thanks! Looks great!

TrueBrain commented 4 years ago

No worries about the speed; you are faster than 99% of the repositories I participate in :D Thank you very much for picking this up so quickly!