Skip to content

Conversation

@amulet1
Copy link
Collaborator

@amulet1 amulet1 commented Dec 23, 2025

This is a proposed fix to #12:

I made getType() method private as it looks like this is not used anywhere outside of Horde_Form. And this is the only place where the named parameters could be passed.

I am using params from value returned by about(). I had to adjust it in a few places (non-critical change) to match original parameters names.

Later we could also add support for defaults by adding them to array returned by about() (and should not this method be static?). It is also possible to implement parameters type checks.

With the migration to V3 this will all be reworked, anyway.

@amulet1 amulet1 requested a review from TDannhauer December 23, 2025 00:34
@amulet1
Copy link
Collaborator Author

amulet1 commented Dec 23, 2025

See also #11 and horde/turba#21.

@amulet1 amulet1 self-assigned this Dec 23, 2025
@amulet1 amulet1 linked an issue Dec 23, 2025 that may be closed by this pull request
@amulet1 amulet1 force-pushed the amulet1-issue-12 branch 2 times, most recently from a176f9b to b683314 Compare December 23, 2025 20:57
@amulet1 amulet1 changed the title DRAFT: fix: getType(): flatten named parameters before calling init() getType(): flatten named parameters before calling init() Dec 24, 2025
@amulet1 amulet1 marked this pull request as ready for review December 24, 2025 15:44
@amulet1
Copy link
Collaborator Author

amulet1 commented Dec 24, 2025

@TDannhauer Please merge.

@amulet1 amulet1 marked this pull request as draft December 24, 2025 16:11
@amulet1
Copy link
Collaborator Author

amulet1 commented Dec 24, 2025

See also #11 and horde/turba#21.

@amulet1 amulet1 marked this pull request as ready for review December 24, 2025 16:16
@TDannhauer
Copy link
Contributor

hi @amulet1 , is your proposal following approach A) or B) from the issue?

I suggest we have a short call - maybe tomorrow - to discuss the approaches , pros and cons to select a road to go.
Sorry, as I am not a native PHP developer, i can not discuss solely based on code...

@amulet1
Copy link
Collaborator Author

amulet1 commented Dec 27, 2025

I chose approach (A) due to several reasons:

  • one simple change in getType(), and it also allows to simplify init() methods
  • no need to undo anything
  • possibility to add support for default values (and further simplify init())
  • possibility to add parameter type checks

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.

Passing named parameters to init()

3 participants