Page MenuHomePhabricator

Page Forms: add parsing to "values from namespace/category" and "unique for namespace/category"
Closed, ResolvedPublic

Description

If you specify "values=" in a form field in a form definition, Page Forms uses the MediaWiki parser to parse the value of that parameter. However, it doesn't do that for "values from namespace=" and "values from category", and it would be better if it did - and the same is true for the parameters "unique for namespace" and "unique for category". I believe all of these can be done by modifying one file.

Event Timeline

Hi, @Yaron_Koren
I am interested in this micro task, is it still available?!

Hello @Yaron_Koren
I've been searching for this micro task for more than hours, and I don't even know if I do It right.

First of all I tried to understand class Parser in includes/parser/Parser.php but I am really confused.
The Documentation here doesn't refer to it as a class, instead there are ParserCache, ParserDiffTest, ParserFactory, ParserOptions and ParserOutput . I think the start may be in ParserFactory or ParserOutput.

Also, searching in the extension's files, leaded me to class PFParserFunctions in extensions/PageForms/includes/PF_ParserFunctions.

Some exploring leaded me to MagicWord and MagicWordFactory (especially the function MagicWordFactory::get($id)).

But I still confused about the right start point to get it done. So, I hope you may help me through the code.

@AmrEl-Absy - if you fully read the task description, there are a few hints in there about how this can best be done...

I think it is a hard puzzle 😃
I figured out my first mistake, I was searching for "value=" not "values=", It leaded me to PF_Template.php on Line 343 and $this->mTemplateFields but it didn't help me.
My restrict problem that I don't know what the mechanism of parsing is, Searching for it leaded me to a really confusing numbers. The class Parser has more than dozen of dozens of functions, more than 50 files called Parser. So, Searching manually in those files and classes is a bad idea. Also, the documentation didn't help me well as it did in the previous microtask.

I guess it depends on .json files that calls some functions, but even this can't help me well.

I've tried a lot but I didn't get the hints. So, Kindly guide me through the right start.

Okay, sorry - maybe the hints were not as helpful as I thought they were. Here's another hint: the file that needs to be changed is /includes/PF_FormField.php.

Finally, I got it.
After fully understanding of the function PFFormField::newFromFormFieldTag(), I found this block of code.

270	} elseif ( $sub_components[0] == 'values' ) {
271		// Handle this one only after
272		// 'delimiter' has also been set.
273		$values = $parser->recursiveTagParse( $sub_components[1] );
	...
280	} elseif ( $sub_components[0] == 'values from category' ) {
281		$valuesSource = $sub_components[1];
282		global $wgCapitalLinks;
283		if ( $wgCapitalLinks ) {
284			$valuesSource = ucfirst( $valuesSource );
285		}
286		$valuesSourceType = 'category';
	...
290	} elseif ( $sub_components[0] == 'values from namespace' ) {
291		$valuesSourceType = 'namespace';
292		$valuesSource = $sub_components[1];
293	}
...
296	} elseif ( $sub_components[0] == 'unique for category' ) {
297		$f->mFieldArgs['unique'] = true;
298		$f->mFieldArgs['unique_for_category'] = $sub_components[1];
299	} elseif ( $sub_components[0] == 'unique for namespace' ) {
300		$f->mFieldArgs['unique'] = true;
301		$f->mFieldArgs['unique_for_namespace'] = $sub_components[1];

and I think it works very well. So, either I've misunderstood the task, or the task is already works fine.
What do you see about that?!

I've forgot to mention this condition statement

if ( $valuesSourceType !== null ) {
	$f->mPossibleValues = PFValuesUtils::getAutocompleteValues( $valuesSource, $valuesSourceType );
	if ( in_array( $valuesSourceType, [ 'category', 'namespace', 'concept' ] ) ) {
		global $wgPageFormsUseDisplayTitle;
		$f->mUseDisplayTitle = $wgPageFormsUseDisplayTitle;
	}
}

I've tried to debug it already, and the variable $f->mPossibleValues stores the elements or members (or whatever values can be called) of the category or namespace that is passed, which I think tells us the parameters "values from namespace=" and "values from category" work very well.

Did I misunderstand anything?!

The issue is the parsing (or lack of parsing). So, for example, "values={{ucfirst:a,b,c}}" in a field tag in a form definition will work well, but "values from category={{ucfirst:books}}" will not.

I've tried values={{ucfirst:a,b,c}} but I don't think it works well. It produces only the first element with ucfirst (A, b, c). and if I want the other elements to be applied with ucfirst it should go like this values={{ucfirst:a}},{{ucfirst:b}},{{ucfirst:c}}
I tried some other cases (like values=ucfirst{{a,b,c}} and values={{ucfirst:a,ucfirst:b}} etc) but all of them don't work.

Is it also a bug needed to be handled or it should go in this exact way?!

Right - {{ucfirst:a,b,c}} doesn't really produce useful output, but it works; and I think the other example I gave doesn't work at all.

Yes, the other example doesn't work at all, but I found a solution for it.
I am working on testing it, and then I will upload it to review.

I think after finishing it, I will be glad to work on {{ucfirst:a,b,c}} if you see it is a good idea.

Change 581631 had a related patch set uploaded (by Amr El-Absy; owner: Amr El-Absy):
[mediawiki/extensions/PageForms@master] Add parsing to "Values from namespace/category"

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

This is a step in the right direction - but what about "unique for ..."?

Also, what would you do for {{ucfirst}}?

Change 581660 had a related patch set uploaded (by Amr El-Absy; owner: Amr El-Absy):
[mediawiki/extensions/PageForms@master] Add Paring to "unique for namespace/category"

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

This is a step in the right direction - but what about "unique for ..."?

Also, what would you do for {{ucfirst}}?

I was working in unique, but I thought they are different, so I separated them into two changes. Would you think they should be in one?!

{{ucfirst}} ?! I am trying to debug it now, but I saw to publish the first change and finish it, and then starting with this.

Well, one patch or two, it doesn't really matter. If you want to do two, that's fine.

Change 581631 merged by jenkins-bot:
[mediawiki/extensions/PageForms@master] Add parsing to "Values from namespace/category/concept"

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

Change 581660 merged by jenkins-bot:
[mediawiki/extensions/PageForms@master] Add parsing to "unique for namespace/category/concept"

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