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

Add @:message.follow #11448

Open
wants to merge 1 commit into
base: development
Choose a base branch
from
Open

Add @:message.follow #11448

wants to merge 1 commit into from

Conversation

Simn
Copy link
Member

@Simn Simn commented Dec 26, 2023

I sometimes have some sort of helper typedefs that I don't actually want users to see in error messages and such. With this metadata, typedefs are followed away in the internal printing:

@:message.follow
private typedef Apocalypse = String;

function main() {
	var a:Apocalypse = 1; // Int should be String
}

@Simn
Copy link
Member Author

Simn commented Dec 26, 2023

Now I wonder if this is too narrow. How would people feel about something more generic that allows e.g. this:

@:message.message("$T1 | $T2")
abstract EitherType<T1, T2>(Dynamic) from T1 to T1 from T2 to T2 {}

Just a custom string with some predefined $-replacements.

@kLabz
Copy link
Contributor

kLabz commented Dec 26, 2023

Couldn't both exist? Then adding @:message.follow would be a good start

(not sure how useful @:message.message could be..)

@Simn
Copy link
Member Author

Simn commented Dec 26, 2023

Well, I picked EitherType as an example because I greatly prefer error: dot.EdgeAttribute | dot.NodeAttribute | dot.GraphAttribute should be dot.ClusterAttribute over error: haxe.extern.EitherType<dot.EdgeAttribute, haxe.extern.EitherType<dot.NodeAttribute, dot.GraphAttribute>> should be dot.ClusterAttribute. But then again I'm sure that somebody will complain that we shouldn't print it like that because we don't support it in syntax like that.

@back2dos
Copy link
Member

back2dos commented Dec 26, 2023

Seems to be cutting very close to the proposed @:displayName.

Also, @:message.follow or even @:message.message seems an odd description for that EitherType use case.

What's desired in both cases is user defined type formatting.

@RblSb
Copy link
Member

RblSb commented Dec 26, 2023

Is this possible to think about some meta to allow writing of EitherType<dot.EdgeAttribute, dot.NodeAttribute, dot.GraphAttribute> syntax too? (offtopic, but also part of readability problem, i think)

@Simn
Copy link
Member Author

Simn commented Dec 26, 2023

Ah right, I knew we discussed this somewhere before. I guess @:displayName does make a bit more sense.

I don't want to add random shorthands for EitherType unless we decide to actually support | in syntax. The problem with that is that I'm not sure it should really be reserved for something random like EitherType. But on the other hand I don't really want to support any other notion of union types, so I don't know.

@skial skial mentioned this pull request Dec 30, 2023
1 task
@back2dos
Copy link
Member

back2dos commented Jan 7, 2024

I don't want to add random shorthands for EitherType unless we decide to actually support | in syntax. The problem with that is that I'm not sure it should really be reserved for something random like EitherType.

Agreed. I would even say it should not, because | very commonly implies type unions and EitherType is vaguely related at best.

For starters "either" implies a disjunctive union. And if we only consider disjunct types (which is how EitherType is typically used), we still have the fact that EitherType<A, B> unifies with EitherType<A, C> both ways. At the same time EitherType<A, B> is not even really the same as EitherType<B, A> and with nested EitherTypes things become weirder still. Proper type unions are flattened, ordered and deduplicated. And maybe more.

FWIW I'd go with $T1 or $T2. It is equally terse and communicates the same idea, but using human language is less misleading here.

But on the other hand I don't really want to support any other notion of union types, so I don't know.

Since adding unions is most likely a breaking change, I think now is the moment to reconsider it. At least for externs it would be very practical (dts2hx could make good use of it), and it could be restricted to that, not unlike haxe.extern.Rest and @:overload were. If we had first class support for unions, then I suspect it might also help in overload / cast resolution in some cases.

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.

4 participants