Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds SemanticFormsSelect extension #248

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

vedmaka
Copy link
Collaborator

@vedmaka vedmaka commented May 31, 2023

@vedmaka vedmaka requested review from jeffw16 and yaronkoren May 31, 2023 18:11
@github-actions
Copy link

🐳 The image based on 9017fb93 commit has been built with 1.39.1-20230531-248 tag as ghcr.io/canastawiki/canasta:1.39.1-20230531-248

@yaronkoren
Copy link
Member

@vedmaka - what feature(s) of SemanticFormsSelect do you think are useful, that are not already in Page Forms? Ironically, you yourself added some SMW querying syntax to Page Forms a few months ago, which I thought made SFS much less necessary.

@vedmaka
Copy link
Collaborator Author

vedmaka commented Jun 1, 2023

The query feature that was added to PageForms is yet quite limited in comparison to SFS. For example, the feature does not support printouts or any ask params like mainlabel. I am not a fan of SFS and, probably, I'd avoid adding SFS, but it's needed by WikiWorks client being migrated to Canasta. Also due to the extension nature (Composer autoload), it can't be mounted through bind on the stack level

Worth to also note separately that the semantic_query [1] feature is for remote autocompletion-enabled inputs which is a bit different use-case

  1. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageForms/+/860983

@yaronkoren
Copy link
Member

Well, for non-autocompletion input, like dropdowns, couldn't you just do values={{#ask:....|format=list}} ?

Anyway, I think the most persuasive argument for adding SFS to Canasta is that it requires Composer for installation - which means that it would be difficult to add it separately, given the existing issues with Composer add-ons. Although, on that note - I am surprised to see that this current patch adds SFS via both Git and Composer. Are those both necessary?

@vedmaka
Copy link
Collaborator Author

vedmaka commented Jun 2, 2023

Well, for non-autocompletion input, like dropdowns, couldn't you just do values={{#ask:....|format=list}} ?

I don't think so, SFS allows using dependent fields values in a query runtime, which is not the case for values={{#ask:

Although, on that note - I am surprised to see that this current patch adds SFS via both Git and Composer. Are those both necessary?

Actually, it's not both, the extension is added via Git, but it still needs composer.json to be merged via merge-plugin for autoloading to work. The SFS deps are quite weirdly composed though, so in fact SFS can't be installed via Composer require

@yaronkoren
Copy link
Member

Right, I know about the dynamic/dependent part of SFS - I think it's not that important, given that Page Forms has "values dependent on", but maybe it has some specific uses that "values dependent on" cannot handle. Anyway, I still think the key aspect is that SFS has (for no good reason) a Composer requirement - and given Canasta's incomplete support for Composer, that seems to make SFS inclusion in Canasta necessary.

I'm still curious about the Composer/Git thing, though. What do you mean by "autoloading"? And what would happen if you just downloaded SFS via Git, then added a wfLoadExtension() call for it?

@vedmaka
Copy link
Collaborator Author

vedmaka commented Jun 3, 2023

I did not use values dependent on much, so I am not quite sure how good a replacement for SFS it can be (I guess it still can't be a full replacement because SFS allows using any custom semantic query). Either way, the point is that even if in this particular client case the SFS can be replaced with values dependent on it would mean all the client forms to be reworked, which is out of the scope for now.

For the "autoloading" I am referring to the autoload section of the SFS composer.json https://github.com/SemanticMediaWiki/SemanticFormsSelect/blob/master/composer.json#L53, without inclusion into the merge-plugin the extension simply won't work because necessary classed won't be loaded

@vedmaka vedmaka requested a review from pastakhov June 4, 2023 15:48
@yaronkoren
Copy link
Member

Well, whatever benefits SFS provides (I still think they are minimal), the fact that it requires Composer for installation makes, in my opinion, a strong reason for including it. Hopefully Composer support will be fixed before the next major Canasta version, and then we can discuss getting rid of SFS (I didn't say get rid of it, I said discuss it), but for now I think it makes sense to include it. So this patch looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants