Core: Remove skip_cr argument from String#110867
Conversation
Repiteo
force-pushed
the
core/deprecate-skip-cr
branch
from
b1bcdfd to
8528fdd
Compare
Repiteo
force-pushed
the
core/deprecate-skip-cr
branch
from
8528fdd to
687d035
Compare
|
Maybe I'm wrong but the PR's title is a bit of a misnomer? "Deprecation" would somewhat implies the parameter is still available but superfluous. Instead it has been removed entirely. |
Repiteo
force-pushed
the
core/deprecate-skip-cr
branch
2 times, most recently
from
e7ab52b to
cb94b7d
Compare
Repiteo
changed the title
skip_cr argumentskip_cr argument from String
Ahh right, that would be correct. I did add a compat method for a function which took in |
Ivorforce
left a comment
Ivorforce
left a comment
There was a problem hiding this comment.
Makes sense to me! Line endings are not a property of the encoding, so the parameter was misplaced.
Repiteo
force-pushed
the
core/deprecate-skip-cr
branch
from
cb94b7d to
90e2cda
Compare
There was a problem hiding this comment.
I really hope we don't have many users who make use of the p_skip_cr option. If we do, we'll have to consider reverting this PR.
However again, in principle, I wouldn't expect the encoding function to handle line endings, so this should be a step in the right direction.
Repiteo
force-pushed
the
core/deprecate-skip-cr
branch
from
90e2cda to
ceb2e21
Compare
Mickeon
left a comment
Mickeon
left a comment
There was a problem hiding this comment.
This gets a personal approval from me because, even just by reading the documentation during localization, I thought it was really, really weird to have this parameter.
Repiteo
force-pushed
the
core/deprecate-skip-cr
branch
from
ceb2e21 to
1f64d1e
Compare
Repiteo
force-pushed
the
core/deprecate-skip-cr
branch
from
1f64d1e to
f6fc2f4
Compare
`skip_cr` argument has been removed: godotengine/godot#110867
5 participants
In a recent core meeting, we discussed removing the
p_skip_crargument from the String contructors because something that specific shouldn't be tied to something so general. This ended up being a bit more involved than I expected, as this argument also applied to aFileAccessfunction, which also needed to be removed. Consequently, theget_line()function had to be expanded to account for\rline endings, and the tests were adjusted to accomodate that