Make python params positional or named#424
Conversation
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
| * [Slash] is standalone rather than a prefix, and [Star] can go either way, but | ||
| * keeping them all together still helps with organization. | ||
| */ | ||
| enum ArgPrefix = None | Slash | Star | DoubleStar; |
| & ((defaultValue%Expr? => "=" & defaultValue) || ()); | ||
| Arg requires `prefix == ArgPrefix.None || defaultValue == null`; | ||
| Arg requires `arg != null || prefix == ArgPrefix.Slash || prefix == ArgPrefix.Star`; | ||
| Arg requires `arg != null || (annotation == null && defaultValue == null)`; |
There was a problem hiding this comment.
This now also allows standalone star to separate out named-only params, but we don't currently generate that.
And I'm mixed on enforcing these rules since it feels somewhat like overhead when we also have funtests and such, but I felt like trying to conform to validation being done here already.
There was a problem hiding this comment.
Ok, so slash needs to appear on its own in a parameter list.
What would it look like if instead of the below, Arguments had named arguments before positional arguments, so the / is still on its own in the comma separated list, but the count of children still matches the count of arguments (modulo * and **).
Arguments ::= args%Arg*", ";
There was a problem hiding this comment.
What would it look like if instead of the below, Arguments had named arguments before positional arguments, so the
/is still on its own in the comma separated list, but the count of children still matches the count of arguments (modulo*and**).
Do you mean like this that I linked to in a different comment?
If so, here's what was trouble there:
- I had to navigate separate param lists
- I couldn't figure out how to prevent a comma after an ending slash
- It doesn't jive well with the
*vs*argsoption of python semi-rest/named-only-divider thinking - It required changing more code to support it
- It didn't fit in as well with the existing star and starStar token usage
| CallArg requires `prefix == ArgPrefix.None || arg == null`; | ||
| // This ArgPrefix is used by Arg. | ||
| CallArg requires `prefix != ArgPrefix.Star`; | ||
| CallArg requires `prefix != ArgPrefix.Slash`; |
There was a problem hiding this comment.
You actually can use star prefix on call args, but slash is only used in defs (and only standalone).
| if (doubleStar) { | ||
| return "$arg: nothing can follow **keywords" | ||
| } | ||
| slash = slash || arg.prefix == Py.ArgPrefix.Slash |
There was a problem hiding this comment.
Again, aligning with existing validation, even though I think it's mostly overkill.
|
|
||
| fun Py.Arguments.Companion.simple(pos: Position, names: List<String>): Py.Arguments = | ||
| Py.Arguments(pos, names.map { Py.Arg(pos, it) }) | ||
| Py.Arguments(pos, args = names.map { Py.Arg(pos, it) }) |
There was a problem hiding this comment.
Not actually needed here. I needed it for an earlier version of positional-only params on this branch, and I never reverted, but it's also not terrible, so I'm inclined to keep it.
| | def identity(this_2, u_11: 'U_3', f_12: 'Callable3[[U_3], T_1]', /) -> 'T_1': | ||
| | return f_12(u_11) | ||
| | def __init__(this_5) -> None: | ||
| | def __init__(this, /) -> None: |
There was a problem hiding this comment.
I changed all constructor params to render pretty and didn't bother to ask the this/self to stay ugly. And it should still stay positional-only, even if it would only matter in syntax like Thing.__init__(this=...), which I tested as otherwise working.
And I'm also ok still saying this instead of self for now, but we could change to self in the future if we verify against other params being called self.
| | def __init__(this_2, radix_15: 'int3') -> None: | ||
| | this_2.radix_7 = radix_15 | ||
| | def __init__(this, /, radix: 'int3') -> None: | ||
| | this.radix_7 = radix |
There was a problem hiding this comment.
Example constructor with non-this arg. And maybe uninteresting for single-param case like this, but you still now can say IntMaker(radix=...), which should still be fine since temper would allow { class: IntMaker, radix: ... }.
| | def radix(this_24, /) -> 'int3': | ||
| | return this_24.radix_7 | ||
| |def crazy_sum(int_maker_16: 'IntMaker', int_17: 'int64_24', string_18: 'str4') -> 'int3': | ||
| |def crazy_sum(int_maker_16: 'IntMaker', int_17: 'int64_24', string_18: 'str4', /) -> 'int3': |
There was a problem hiding this comment.
And we could/should presumably make all params pretty now, given the / to prevent named use, but I didn't do that in this pr, since I didn't want to think about new name conflicts.
| | prop_name_9: 'str0' | ||
| | __slots__ = ('prop_name_9',) | ||
| | def __init__(blah_12: 'str0') -> None: | ||
| | def __init__(blah: 'str0', /) -> None: |
There was a problem hiding this comment.
This case is from hand-crafted tmpl that gives this the name blah.
| is TmpL.Import -> if (ignoreImport) TmpL.IdReach.Internal else TmpL.IdReach.External | ||
| is TmpL.Import if !ignoreImport -> TmpL.IdReach.External | ||
| else -> TmpL.IdReach.Internal | ||
| } |
There was a problem hiding this comment.
The constructor case above is what makes constructor params pretty now. I also refactored the logic a bit for the import case.
Signed-off-by: Tom <tom@temper.systems>
2 participants
Uh oh!
There was an error while loading. Please reload this page.