search

PaulSonOfLars / gotgbot

Autogenerated Go wrapper for the telegram API. Inspired by the python-telegram-bot library.
MIT License
509 stars 114 forks source link

avoid unnecessary Sprintf #76

Closed vassilit closed 1 year ago

vassilit commented 1 year ago

Sprintf should be less efficient than Join, as we are effectively joining two strings by a separator. The «proper» way could have been to use the net/url package, but Join is simpler.

vassilit commented 1 year ago

You're right, let's just use url.JoinPath here; that seems much cleaner, and removes the need for the strings.Suffix!

Yes, but we should modify go.mod also, because url.JoinPath appeared in Go 1.19.

I guess it could cause issues if the data.urlPath contained extra slashes

if data.urlPath contain extra slashes right now, this is already a problem, as we don't know how the telegram server will handle ".././../" in URLs. We could use url.PathEscape on the urlPath part.

PaulSonOfLars commented 1 year ago

Yes, but we should modify go.mod also, because url.JoinPath appeared in Go 1.19.

Ahh, thats why. Makes sense, guess we should wait on that for a bit then. Shame.

if data.urlPath contain extra slashes right now, this is already a problem, as we don't know how the telegram server will handle ".././../" in URLs. We could use url.PathEscape on the urlPath part.

True. Eh, I'm OK with calling that user error. We probably shouldnt mess with that input too much, since itll be used by the user in other places.