-
Notifications
You must be signed in to change notification settings - Fork 385
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
Fix typos #865
base: master
Are you sure you want to change the base?
Fix typos #865
Conversation
For the willing contributor: [files]
extend-exclude = [
".git/",
"src/Parse/Date.php",
"src/Misc.php",
"demo/for_the_demo/sifr.js",
"demo/for_the_demo/source_files/sIFR-r245/",
"demo/for_the_demo/favicons/",
]
ignore-hidden = false
[default]
extend-ignore-re = [
"return '<!DOCTYPE html.*'",
"'curren;?' =>",
"“Unio,",
]
[default.extend-identifiers]
# TODO
"datas" = "datas"
"nsIInterfaceRequestor" = "nsIInterfaceRequestor" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but renaming the global $non_ascii_octects
could be a breaking change. We have to check this first.
@@ -292,13 +292,13 @@ public static function change_encoding(string $data, string $input, string $outp | |||
|
|||
// We fail to fail on non US-ASCII bytes | |||
if ($input === 'US-ASCII') { | |||
static $non_ascii_octects = ''; | |||
if (!$non_ascii_octects) { | |||
static $non_ascii_octets = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming this global variable could be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I do not know how to handle this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean? Is there some way to access the static variable outside the scope of this function?
I do not see any mention in the docs and reusing the name in a child method seems to create independent variable: https://3v4l.org/hKPjc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean I do not know to handle a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant that I do not see how this could be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant that I do not see how this could be a breaking change.
You are right. I missed the assignment of the empty string = '';
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable was introduced Dec 2007 with a9633c8
We should remove the global
statement while renaming the variable.
Update: Sorry, I totally messed up static
and global
. 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it is just an optimization to avoid creating the list multiple times. But we could just use hard-code it to "\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff"
But that does not have to go to this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about a private static property in the Misc
class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have a strong preference. On one hand, moving it to the class would allow us to use const
. On the other hand, the value is only used by a single function so it makes sense to me for it to be defined locally.
Is it this tool https://github.com/crate-ci/typos? Might be nice to add it to CI. |
Found few misspellings.