-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Refactor git attributes #29356
Refactor git attributes #29356
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both if
s 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
.
language = AttributeToString(attrs, AttributeGitlabLanguage) | ||
if language.Has() { | ||
raw := language.Value() | ||
if idx := strings.IndexByte(raw, '?'); idx >= 0 { |
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should add a comment for it.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]() |
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.
See two comments above
language = AttributeToString(attrs, AttributeGitlabLanguage) | ||
if language.Has() { | ||
raw := language.Value() | ||
if idx := strings.IndexByte(raw, '?'); idx >= 0 { |
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.
Perhaps we should add a comment for it.
No description provided.