Skip to content

Comments

Fix translate URL#380

Merged
ximion merged 1 commit intoximion:masterfrom
JakobDev:translink
Jan 26, 2022
Merged

Fix translate URL#380
ximion merged 1 commit intoximion:masterfrom
JakobDev:translink

Conversation

@JakobDev
Copy link
Contributor

While looking at the source, I saw that UrlTranslate is defined in Header as part of UrlKind but not set.

btw:
Is there any reason why it is named UrlTranslate and not UrlKindTranslate like the other?

@ximion
Copy link
Owner

ximion commented Jan 25, 2022

Wow, something went massively wrong here... Good catch!

@JakobDev
Copy link
Contributor Author

Should I rename UrlTranslate to UrlKindTranslate? It never worked and there are no Issues about it, so I don't think it is used in any code.

@ximion ximion merged commit eac0f4d into ximion:master Jan 26, 2022
ximion added a commit that referenced this pull request Jan 26, 2022
ximion added a commit that referenced this pull request Jan 26, 2022
The Qt way is of course nicer, but it's yet another place to keep a
string table up to date, so calling the C library for that conversion is
a lot more convenient in the long run and easier to maintain. And unless
people are doing insane things with the ASQt API, the additional
overhead should not be noticeable.

CC: #380
@ximion
Copy link
Owner

ximion commented Jan 26, 2022

Should I rename UrlTranslate to UrlKindTranslate? It never worked and there are no Issues about it, so I don't think it is used in any code.

No, we can't do that as that would still be an API break (the enum value was valid afterall and some client could have used it in a Component::url call). I changed the code to properly deprecated the mistyped one in f336132, so this should work now :-) (and with the next API/ABI break we can then remove this enum value fully)
Thank you for noticing this issue and providing a patch!

@JakobDev JakobDev deleted the translink branch March 17, 2022 15:46
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