- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.2k
          Enable gocritic equalFold and fix issues
          #34952
        
          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
Changes from 2 commits
573b40d
              f52fd70
              3282b02
              13cb941
              2e8c7d4
              7d17475
              8e83d6c
              7e914c8
              c700814
              edc11bc
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -91,8 +91,7 @@ func (r *stripRenderer) processAutoLink(w io.Writer, link []byte) { | |
| } | ||
|  | ||
| // Note: we're not attempting to match the URL scheme (http/https) | ||
| host := strings.ToLower(u.Host) | ||
| if host != "" && host != strings.ToLower(r.localhost.Host) { | ||
| if u.Host != "" && !strings.EqualFold(u.Host, r.localhost.Host) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Host should be ASCII only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hostnames can contain unicode potentially: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, backend is ASCII-only, see the Punycode 
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you certain that domains will enter in their punycode form into this function? I am not and therefore it's better to use unicode-aware functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. | ||
| // Process out of band | ||
| r.links = append(r.links, linkStr) | ||
| return | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -62,11 +62,11 @@ func (c logCompression) IsValid() bool { | |
| } | ||
|  | ||
| func (c logCompression) IsNone() bool { | ||
| return strings.ToLower(string(c)) == "none" | ||
| return strings.EqualFold(string(c), "none") | ||
|          | ||
| } | ||
|  | ||
| func (c logCompression) IsZstd() bool { | ||
| return c == "" || strings.ToLower(string(c)) == "zstd" | ||
| return c == "" || strings.EqualFold(string(c), "zstd") | ||
|         
                  silverwind marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved         
                  silverwind marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
| } | ||
|  | ||
| func loadActionsFrom(rootCfg ConfigProvider) error { | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -12,8 +12,7 @@ import ( | |
| // SliceContainsString sequential searches if string exists in slice. | ||
| func SliceContainsString(slice []string, target string, insensitive ...bool) bool { | ||
| if len(insensitive) != 0 && insensitive[0] { | ||
| target = strings.ToLower(target) | ||
| return slices.ContainsFunc(slice, func(t string) bool { return strings.ToLower(t) == target }) | ||
| return slices.ContainsFunc(slice, func(t string) bool { return strings.EqualFold(t, target) }) | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds dangerous to just assume that for a utility function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In real world, we have used to assume "case-insensitive" means ASCII-only. That's why I believe Golang's  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I think we are blowing this out of proportion. Nowadays it should be assumed that strings contain unicode and not ASCII and it's good to use unicode-aware case convertions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 But not for "backend logic" like "path handling", "name comparing" and "protocol parsing" (we have discussed it in another PR) We have strictly required "username" to be "ASCII-only", it doesn't make sense to use "Unicode case-insensitive functions" to compare a user's input to the ASCII-only internal username. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I will leave this case unchanged as its a generic utility function. | ||
| } | ||
|  | ||
| return slices.Contains(slice, target) | ||
|  | ||

Uh oh!
There was an error while loading. Please reload this page.