Refactor git attributes #29356
Refactor git attributes #29356
Conversation
KN4CK3R
added
the
type/refactoring
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/L
| if !isVendored.Has() { | ||
| isVendored = optional.Some(analyze.IsVendor(diffFile.Name)) | ||
| } | ||
| if !gotGenerated { | ||
| diffFile.IsGenerated = analyze.IsGenerated(diffFile.Name) | ||
| diffFile.IsVendored = isVendored.Value() | ||
|
|
||
| if !isGenerated.Has() { |
There was a problem hiding this comment.
Both ifs are changed to be consistent with the repo language stats. If the .gitattribute tells a file is not vendored, this should not be overridden by analyse.IsVendor.
GiteaBot
added
lgtm/need 1
| language = AttributeToString(attrs, AttributeGitlabLanguage) | ||
| if language.Has() { | ||
| raw := language.Value() | ||
| if idx := strings.IndexByte(raw, '?'); idx >= 0 { |
There was a problem hiding this comment.
I see it is from old code ... I am quite curious that why checking ? here ... feel free to add a comment or dismiss this question if it is still unclear.
There was a problem hiding this comment.
I've seen in the tests that the format can be something like
*.md gitlab-language=Go?parent=Markdown.
But yes, it is a weird mechanism…
There was a problem hiding this comment.
Perhaps we should add a comment for it.
There was a problem hiding this comment.
The syntax is shown here: https://docs.gitlab.com/ee/user/project/highlighting.html#override-syntax-highlighting-for-a-file-type
You can also extend the highlighting with Common Gateway Interface (CGI) options, such as:
Whatever that means.
| generated, has := attrs["linguist-generated"] | ||
| ctx.Data["IsGenerated"] = has && (generated == "set" || generated == "true") | ||
| ctx.Data["IsVendored"] = git.AttributeToBool(attrs, git.AttributeLinguistVendored).Value() | ||
| ctx.Data["IsGenerated"] = git.AttributeToBool(attrs, git.AttributeLinguistGenerated).Value() |
There was a problem hiding this comment.
By the way, since we now support more attributes, shouldn't they be added here as well?
But perhaps in a follow up PR…
| language = AttributeToString(attrs, AttributeGitlabLanguage) | ||
| if language.Has() { | ||
| raw := language.Value() | ||
| if idx := strings.IndexByte(raw, '?'); idx >= 0 { |
There was a problem hiding this comment.
I've seen in the tests that the format can be something like
*.md gitlab-language=Go?parent=Markdown.
But yes, it is a weird mechanism…
| @@ -1181,41 +1182,30 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi | |||
|
|
|||
| for _, diffFile := range diff.Files { | |||
|
|
|||
| gotVendor := false | |||
| gotGenerated := false | |||
| isVendored := optional.None[bool]() | |||
| language = AttributeToString(attrs, AttributeGitlabLanguage) | ||
| if language.Has() { | ||
| raw := language.Value() | ||
| if idx := strings.IndexByte(raw, '?'); idx >= 0 { |
There was a problem hiding this comment.
Perhaps we should add a comment for it.
GiteaBot
added
lgtm/done
yardenshoham
added
the
reviewed/wait-merge
GiteaBot
removed
the
reviewed/wait-merge
No description provided.