From ee39c5812028d926c650c9e8fce9c2e6364adb0a Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 26 Feb 2024 12:52:59 +0100 Subject: [PATCH 1/2] Convert linguist attribute handling to optional.Option Based on @KN4CK3R's work in gitea#29267. This drops the custom `LinguistBoolAttrib` type, and uses `optional.Option` instead. I added the `isTrue()` and `isFalse()` (function-local) helpers to make the code easier to follow, because these names convey their goal better than `v.ValueorDefault(false)` or `!v.ValueOrDefault(true)`. Signed-off-by: Gergely Nagy --- modules/git/repo_attribute.go | 21 +++++++++++ modules/git/repo_language_stats.go | 12 ------- modules/git/repo_language_stats_gogit.go | 42 +++++++++++----------- modules/git/repo_language_stats_nogogit.go | 42 +++++++++++----------- 4 files changed, 63 insertions(+), 54 deletions(-) diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 3e09828ec5..674ae1dd75 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -11,6 +11,7 @@ import ( "os" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" ) // CheckAttributeOpts represents the possible options to CheckAttribute @@ -322,3 +323,23 @@ func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeRe return checker, deferable } + +// true if "set"/"true", false if "unset"/"false", none otherwise +func attributeToBool(attr map[string]string, name string) optional.Option[bool] { + if value, has := attr[name]; has && value != "unspecified" { + switch value { + case "set", "true": + return optional.Some(true) + case "unset", "false": + return optional.Some(false) + } + } + return optional.None[bool]() +} + +func attributeToString(attr map[string]string, name string) optional.Option[string] { + if value, has := attr[name]; has && value != "unspecified" { + return optional.Some(value) + } + return optional.None[string]() +} diff --git a/modules/git/repo_language_stats.go b/modules/git/repo_language_stats.go index 7ed2dc1587..c40d6937b5 100644 --- a/modules/git/repo_language_stats.go +++ b/modules/git/repo_language_stats.go @@ -13,18 +13,6 @@ const ( bigFileSize int64 = 1024 * 1024 // 1 MiB ) -type LinguistBoolAttrib struct { - Value string -} - -func (attrib *LinguistBoolAttrib) IsTrue() bool { - return attrib.Value == "set" || attrib.Value == "true" -} - -func (attrib *LinguistBoolAttrib) IsFalse() bool { - return attrib.Value == "unset" || attrib.Value == "false" -} - // mergeLanguageStats mergers language names with different cases. The name with most upper case letters is used. func mergeLanguageStats(stats map[string]int64) map[string]int64 { names := map[string]struct { diff --git a/modules/git/repo_language_stats_gogit.go b/modules/git/repo_language_stats_gogit.go index 558a83af74..da0b209b37 100644 --- a/modules/git/repo_language_stats_gogit.go +++ b/modules/git/repo_language_stats_gogit.go @@ -12,6 +12,7 @@ import ( "strings" "code.gitea.io/gitea/modules/analyze" + "code.gitea.io/gitea/modules/optional" "github.com/go-enry/go-enry/v2" "github.com/go-git/go-git/v5" @@ -53,31 +54,30 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err firstExcludedLanguage := "" firstExcludedLanguageSize := int64(0) + isTrue := func(v optional.Option[bool]) bool { + return v.ValueOrDefault(false) + } + isFalse := func(v optional.Option[bool]) bool { + return !v.ValueOrDefault(true) + } + err = tree.Files().ForEach(func(f *object.File) error { if f.Size == 0 { return nil } - isVendored := LinguistBoolAttrib{} - isGenerated := LinguistBoolAttrib{} - isDocumentation := LinguistBoolAttrib{} - isDetectable := LinguistBoolAttrib{} + isVendored := optional.None[bool]() + isGenerated := optional.None[bool]() + isDocumentation := optional.None[bool]() + isDetectable := optional.None[bool]() if checker != nil { attrs, err := checker.CheckPath(f.Name) if err == nil { - if vendored, has := attrs["linguist-vendored"]; has { - isVendored = LinguistBoolAttrib{Value: vendored} - } - if generated, has := attrs["linguist-generated"]; has { - isGenerated = LinguistBoolAttrib{Value: generated} - } - if documentation, has := attrs["linguist-documentation"]; has { - isDocumentation = LinguistBoolAttrib{Value: documentation} - } - if detectable, has := attrs["linguist-detectable"]; has { - isDetectable = LinguistBoolAttrib{Value: detectable} - } + isVendored = attributeToBool(attrs, "linguist-vendored") + isGenerated = attributeToBool(attrs, "linguist-generated") + isDocumentation = attributeToBool(attrs, "linguist-documentation") + isDetectable = attributeToBool(attrs, "linguist-detectable") if language, has := attrs["linguist-language"]; has && language != "unspecified" && language != "" { // group languages, such as Pug -> HTML; SCSS -> CSS group := enry.GetLanguageGroup(language) @@ -108,11 +108,11 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err } } - if isDetectable.IsFalse() || isVendored.IsTrue() || isDocumentation.IsTrue() || - (!isVendored.IsFalse() && analyze.IsVendor(f.Name)) || + if isFalse(isDetectable) || isTrue(isVendored) || isTrue(isDocumentation) || + (!isFalse(isVendored) && analyze.IsVendor(f.Name)) || enry.IsDotFile(f.Name) || enry.IsConfiguration(f.Name) || - (!isDocumentation.IsFalse() && enry.IsDocumentation(f.Name)) { + (!isFalse(isDocumentation) && enry.IsDocumentation(f.Name)) { return nil } @@ -121,7 +121,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err if f.Size <= bigFileSize { content, _ = readFile(f, fileSizeLimit) } - if !isGenerated.IsTrue() && enry.IsGenerated(f.Name, content) { + if !isTrue(isGenerated) && enry.IsGenerated(f.Name, content) { return nil } @@ -142,7 +142,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err langtype := enry.GetLanguageType(language) included = langtype == enry.Programming || langtype == enry.Markup if !included { - if isDetectable.IsTrue() { + if isTrue(isDetectable) { included = true } else { return nil diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 13876094cc..44235c0a49 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/analyze" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "github.com/go-enry/go-enry/v2" ) @@ -77,6 +78,13 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err firstExcludedLanguage := "" firstExcludedLanguageSize := int64(0) + isTrue := func(v optional.Option[bool]) bool { + return v.ValueOrDefault(false) + } + isFalse := func(v optional.Option[bool]) bool { + return !v.ValueOrDefault(true) + } + for _, f := range entries { select { case <-repo.Ctx.Done(): @@ -91,26 +99,18 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err continue } - isVendored := LinguistBoolAttrib{} - isGenerated := LinguistBoolAttrib{} - isDocumentation := LinguistBoolAttrib{} - isDetectable := LinguistBoolAttrib{} + isVendored := optional.None[bool]() + isGenerated := optional.None[bool]() + isDocumentation := optional.None[bool]() + isDetectable := optional.None[bool]() if checker != nil { attrs, err := checker.CheckPath(f.Name()) if err == nil { - if vendored, has := attrs["linguist-vendored"]; has { - isVendored = LinguistBoolAttrib{Value: vendored} - } - if generated, has := attrs["linguist-generated"]; has { - isGenerated = LinguistBoolAttrib{Value: generated} - } - if documentation, has := attrs["linguist-documentation"]; has { - isDocumentation = LinguistBoolAttrib{Value: documentation} - } - if detectable, has := attrs["linguist-detectable"]; has { - isDetectable = LinguistBoolAttrib{Value: detectable} - } + isVendored = attributeToBool(attrs, "linguist-vendored") + isGenerated = attributeToBool(attrs, "linguist-generated") + isDocumentation = attributeToBool(attrs, "linguist-documentation") + isDetectable = attributeToBool(attrs, "linguist-detectable") if language, has := attrs["linguist-language"]; has && language != "unspecified" && language != "" { // group languages, such as Pug -> HTML; SCSS -> CSS group := enry.GetLanguageGroup(language) @@ -142,11 +142,11 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err } } - if isDetectable.IsFalse() || isVendored.IsTrue() || isDocumentation.IsTrue() || - (!isVendored.IsFalse() && analyze.IsVendor(f.Name())) || + if isFalse(isDetectable) || isTrue(isVendored) || isTrue(isDocumentation) || + (!isFalse(isVendored) && analyze.IsVendor(f.Name())) || enry.IsDotFile(f.Name()) || enry.IsConfiguration(f.Name()) || - (!isDocumentation.IsFalse() && enry.IsDocumentation(f.Name())) { + (!isFalse(isDocumentation) && enry.IsDocumentation(f.Name())) { continue } @@ -179,7 +179,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err return nil, err } } - if !isGenerated.IsTrue() && enry.IsGenerated(f.Name(), content) { + if !isTrue(isGenerated) && enry.IsGenerated(f.Name(), content) { continue } @@ -201,7 +201,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err langType := enry.GetLanguageType(language) included = langType == enry.Programming || langType == enry.Markup if !included { - if isDetectable.IsTrue() { + if isTrue(isDetectable) { included = true } else { continue From ae0635fd61b10184e5075d563050d175adf750dd Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 26 Feb 2024 14:04:09 +0100 Subject: [PATCH 2/2] Correctly support linguist-documentation=false If a documentation file is marked with a `linguist-documentation=false` attribute, include it in language stats. However, make sure that we do *not* include documentation languages as fallback. Added a new test case to exercise the formerly buggy behaviour. Problem discovered while reviewing @KN4CK3R's tests from gitea#29267. Signed-off-by: Gergely Nagy --- modules/git/repo_attribute.go | 7 ------- modules/git/repo_language_stats_gogit.go | 17 +++++++++-------- modules/git/repo_language_stats_nogogit.go | 15 +++++++-------- tests/integration/linguist_test.go | 13 +++++++++++++ 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 674ae1dd75..197d626a4d 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -336,10 +336,3 @@ func attributeToBool(attr map[string]string, name string) optional.Option[bool] } return optional.None[bool]() } - -func attributeToString(attr map[string]string, name string) optional.Option[string] { - if value, has := attr[name]; has && value != "unspecified" { - return optional.Some(value) - } - return optional.None[string]() -} diff --git a/modules/git/repo_language_stats_gogit.go b/modules/git/repo_language_stats_gogit.go index da0b209b37..1276ce1a44 100644 --- a/modules/git/repo_language_stats_gogit.go +++ b/modules/git/repo_language_stats_gogit.go @@ -138,21 +138,22 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err } included, checked := includedLanguage[language] + langType := enry.GetLanguageType(language) if !checked { - langtype := enry.GetLanguageType(language) - included = langtype == enry.Programming || langtype == enry.Markup - if !included { - if isTrue(isDetectable) { - included = true - } else { - return nil - } + included = langType == enry.Programming || langType == enry.Markup + if !included && (isTrue(isDetectable) || (langType == enry.Prose && isFalse(isDocumentation))) { + included = true } includedLanguage[language] = included } if included { sizes[language] += f.Size } else if len(sizes) == 0 && (firstExcludedLanguage == "" || firstExcludedLanguage == language) { + // Only consider Programming or Markup languages as fallback + if !(langType == enry.Programming || langType == enry.Markup) { + return nil + } + firstExcludedLanguage = language firstExcludedLanguageSize += f.Size } diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 44235c0a49..b313e2298f 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -197,25 +197,24 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err } included, checked := includedLanguage[language] + langType := enry.GetLanguageType(language) if !checked { - langType := enry.GetLanguageType(language) included = langType == enry.Programming || langType == enry.Markup - if !included { - if isTrue(isDetectable) { - included = true - } else { - continue - } + if !included && (isTrue(isDetectable) || (langType == enry.Prose && isFalse(isDocumentation))) { + included = true } includedLanguage[language] = included } if included { sizes[language] += f.Size() } else if len(sizes) == 0 && (firstExcludedLanguage == "" || firstExcludedLanguage == language) { + // Only consider Programming or Markup languages as fallback + if !(langType == enry.Programming || langType == enry.Markup) { + continue + } firstExcludedLanguage = language firstExcludedLanguageSize += f.Size() } - continue } // If there are no included languages add the first excluded language diff --git a/tests/integration/linguist_test.go b/tests/integration/linguist_test.go index 12f12f5cb0..4c8ae4bdb3 100644 --- a/tests/integration/linguist_test.go +++ b/tests/integration/linguist_test.go @@ -251,5 +251,18 @@ func TestLinguistSupport(t *testing.T) { assertFileLanguage(t, "/blame/branch/main/foo.c", "Bash") }) }) + + // 10. Marking a file as non-documentation + t.Run("linguist-documentation=false", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + repo, sha, f := prep(t, "README.md linguist-documentation=false\n") + defer f() + + langs := getFreshLanguageStats(t, repo, sha) + assert.Len(t, langs, 2) + assert.Equal(t, "Markdown", langs[0].Language) + assert.Equal(t, "C", langs[1].Language) + }) }) }