search

diamondburned / gotk4

Autogenerated GTK4 bindings for Go
GNU Affero General Public License v3.0
498 stars 19 forks source link

Prep work from my GTK-Doc parser #140

Closed LukeShu closed 2 months ago

LukeShu commented 5 months ago

Last year on Mastodon I teased that I was working on a proper GTK-Doc/GI-DocGen markup parser. (1) That turned out to be a lot bigger task than I originally anticipated, and (2) I stopped working on it for a while.

I'm now back to trying to push it over the finish line.

Here is some prep work that is mostly-unrelated to the bulk of the parser, that I figured I could go ahead and send your way.

LukeShu commented 5 months ago

v2:

diff from v1 to v2 ```patch diff --git a/gir/gir.go b/gir/gir.go index e9f975cf8..5e7b9f293 100644 --- a/gir/gir.go +++ b/gir/gir.go @@ -116,12 +116,17 @@ type PkgRepository struct { // FindGIRFiles finds gir files from the given list of pkgs. func FindGIRFiles(pkgs ...string) ([]string, error) { - girDirs, err := pkgconfig.GIRDir(pkgs...) + girDirs, err := pkgconfig.GIRDirs(pkgs...) if err != nil { return nil, err } visited := make(map[string]struct{}, len(girDirs)) var girFiles []string + // Iterate over `pkgs` instead of over `girDirs` directly, so + // that the order of `girFiles` is predictable based on order + // of the input arguments. At least this was important to me + // for debugability, but I feel like I had another reason that + // I no longer remember. for _, pkg := range pkgs { girDir := girDirs[pkg] if _, ok := visited[girDir]; ok { diff --git a/gir/pkgconfig/pkgconfig.go b/gir/pkgconfig/pkgconfig.go index 05cc08c42..1e15ceab7 100644 --- a/gir/pkgconfig/pkgconfig.go +++ b/gir/pkgconfig/pkgconfig.go @@ -1,5 +1,5 @@ // Copyright (C) 2021, 2023 diamondburned -// Copyright (C) 2023 Luke Shumaker +// Copyright (C) 2023-2024 Luke T. Shumaker // Package pkgconfig provides a wrapper around the pkg-config binary. package pkgconfig @@ -10,17 +10,71 @@ import ( "os/exec" "path/filepath" "strings" + "sync" ) -// Variable returns a map of map[pkg]value containing the given -// variable value for each requested package. It is not an error for -// a variable to be unset or empty; ret[pkg] is an empty string if +// A ValueSource is a function that takes a list of package names and +// returns a map of package-name to some value. +type ValueSource = func(pkgs ...string) (map[string]string, error) + +var pkgConfigVer = sync.OnceValues[string, error](func() (string, error) { + var stdout strings.Builder + cmd := exec.Command("pkg-config", "--version") + cmd.Stdout = &stdout + if err := cmd.Run(); err != nil { + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + return "", fmt.Errorf("pkg-config failed: %w", err) + } + return "", fmt.Errorf("pkg-config failed with status %d:\n%s", + exitErr.ExitCode(), exitErr.Stderr) + } + return strings.TrimRight(stdout.String(), "\n"), nil +}) + +// Values returns a map of package-name to variable-value of the given +// variable name for each requested package. It is not an error for a +// variable to be unset or empty; ret[pkgname] is an empty string if // that package does not have the variable set. -func Variable(varname string, pkgs ...string) (map[string]string, error) { +func Values(varname string, pkgs ...string) (map[string]string, error) { if len(pkgs) == 0 { return nil, nil } + // There are 3 pkg-config implementations that we should work with: + // + // 1. FDO `pkg-config` + // https://www.freedesktop.org/wiki/Software/pkg-config/ + // (last release was 0.29.2 in 2017) + // + // 2. pkgconf 1.8.x `pkg-config` + // https://gitea.treehouse.systems/ariadne/pkgconf/src/branch/stable/1.8.x + // (last release was 1.8.1 in Jan 2023) + // + // 3. pkgconf 1.9.x/2.x `pkg-config` + // https://gitea.treehouse.systems/ariadne/pkgconf/src/branch/master + // (last release was 2.2.0 in Mar 2024) + + // pkgconf 1.9.0+ doesn't let us query more than one package + // at once. 1.9.0-1.9.4 do an inexplicably wrong thing if we + // ask; 1.9.5 and later ignore all packages except for the + // first one. + ver, err := pkgConfigVer() + if err != nil { + return nil, err + } + if len(pkgs) > 1 && (strings.HasPrefix(ver, "1.9.") || strings.HasPrefix(ver, "2.")) { + ret := make(map[string]string, len(pkgs)) + for _, pkg := range pkgs { + single, err := Values(varname, pkg) + if err != nil { + return nil, err + } + ret[pkg] = single[pkg] + } + return ret, nil + } + cmdline := append([]string{"pkg-config", // Don't be opaque when we fail. "--print-errors", @@ -48,17 +102,35 @@ func Variable(varname string, pkgs ...string) (map[string]string, error) { return nil, fmt.Errorf("pkg-config failed with status %d:\n%s", exitErr.ExitCode(), exitErr.Stderr) } + return parseValues(pkgs, cmdline, stdout.String()) +} +// parseValues parses the output of `pkg-config`. It is a separate +// function from [Values] for unit-testing purposes. +func parseValues(pkgs []string, cmdline []string, stdout string) (map[string]string, error) { ret := make(map[string]string, len(pkgs)) - stdoutStr := strings.TrimRight(stdout.String(), "\n") + stdoutStr := strings.TrimRight(stdout, "\n") if stdoutStr == "" { for i := range pkgs { ret[pkgs[i]] = "" } } else { vals := strings.Split(stdoutStr, " ") - if len(vals) != len(pkgs) { - return nil, fmt.Errorf("%v returned %d values, but expected %d", + if len(vals) < len(pkgs) { + // FDO `pkg-config` omits the separating + // spaces for any leading empty values. This + // is likely a bug where the author assumed + // that `if (str->len > 0)` would be true + // after the first iteration, but it isn't + // when the first iteration's `var->len==0`. + // + // https://gitlab.freedesktop.org/pkg-config/pkg-config/-/blob/pkg-config-0.29.2/pkg.c?ref_type=tags#L1061-L1062 + partialVals := vals + vals = make([]string, len(pkgs)) + copy(vals[len(vals)-len(partialVals):], partialVals) + } + if len(vals) > len(pkgs) { + return nil, fmt.Errorf("%v returned %d values, but only expected %d", cmdline, len(vals), len(pkgs)) } @@ -70,11 +142,11 @@ func Variable(varname string, pkgs ...string) (map[string]string, error) { return ret, nil } -// VariableOrElse is like Variable, but if a package has an -// empty/unset value, then that empty value is replaced with the value -// that is returned from the fn function. -func VariableOrElse(varname string, fn func(...string) (map[string]string, error), pkgs ...string) (map[string]string, error) { - ret, err := Variable(varname, pkgs...) +// ValuesOrElse is like [Values], but if a package has an empty/unset +// value, then that empty value is replaced with the value that is +// returned from the elseFn function. +func ValuesOrElse(varname string, elseFn ValueSource, pkgs ...string) (map[string]string, error) { + ret, err := Values(varname, pkgs...) if err != nil { return nil, err } @@ -86,7 +158,7 @@ func VariableOrElse(varname string, fn func(...string) (map[string]string, error } } if len(badPkgs) > 0 { - aug, err := fn(badPkgs...) + aug, err := elseFn(badPkgs...) if err != nil { return nil, err } @@ -98,11 +170,12 @@ func VariableOrElse(varname string, fn func(...string) (map[string]string, error return ret, nil } -// AddPathSuffix takes a function that returns a map[pkg]dir and wraps -// it so that each `dir` is set to `filepath.Join(dir, ...suffix)`. -func AddPathSuffix(fn func(...string) (map[string]string, error), suffix ...string) func(...string) (map[string]string, error) { +// AddPathSuffix takes a [ValueSource] that returns a map of +// package-name to directory, and wraps it so that each `dir` is set +// to `filepath.Join(dir, ...suffix)`. +func AddPathSuffix(inner ValueSource, suffix ...string) ValueSource { return func(pkgs ...string) (map[string]string, error) { - ret, err := fn(pkgs...) + ret, err := inner(pkgs...) if err != nil { return nil, err } @@ -113,40 +186,49 @@ func AddPathSuffix(fn func(...string) (map[string]string, error), suffix ...stri } } -// Prefix returns the install prefix for each requested package, or an -// error if this cannot be determined for any of the packages. Common -// values are "/", "/usr", or "/usr/local". -func Prefix(pkgs ...string) (map[string]string, error) { - return VariableOrElse("prefix", func(pkgs ...string) (map[string]string, error) { +// Prefixes returns a map of package-name to the install prefix for +// each requested package, or an error if this cannot be determined +// for any of the packages. Common values are "/", "/usr", or +// "/usr/local". +// +// Prefixes is a [ValueSource]. +func Prefixes(pkgs ...string) (map[string]string, error) { + return ValuesOrElse("prefix", func(pkgs ...string) (map[string]string, error) { return nil, fmt.Errorf("could not resolve install prefix for packages: %v", pkgs) }, pkgs...) } -// DataRootDir returns the directory for read-only -// architecture-independent data files for each requested package, or -// an error if this cannot be determined for any of the packages. The -// usual value is "${prefix}/share", i.e. "/usr/share" or -// "/usr/local/share". -func DataRootDir(pkgs ...string) (map[string]string, error) { - return VariableOrElse("datarootdir", AddPathSuffix(Prefix, "share"), pkgs...) -} - -// DataDir returns the base directory for package-idiosyncratic +// DataRootDirs returns a map of package-name to the directory for // read-only architecture-independent data files for each requested // package, or an error if this cannot be determined for any of the -// packages. The usual value is "${datarootdir}", i.e. "/usr/share" -// or "/usr/local/share"; this is *not* a per-package directory, -// packages usually install their data to -// "${datadir}/${package_name}/". -func DataDir(pkgs ...string) (map[string]string, error) { - return VariableOrElse("datadir", DataRootDir, pkgs...) +// packages. The usual value is "${prefix}/share", i.e. "/usr/share" +// or "/usr/local/share". +// +// DataRootDirs is a [ValueSource]. +func DataRootDirs(pkgs ...string) (map[string]string, error) { + return ValuesOrElse("datarootdir", AddPathSuffix(Prefixes, "share"), pkgs...) } -// GIRDir returns the directory for GObject Introspection Repository -// XML files for each requested package, or an error if this cannot be +// DataDirs returns a map of package-name to the base directory for +// package-idiosyncratic read-only architecture-independent data files +// for each requested package, or an error if this cannot be // determined for any of the packages. The usual value is -// "${datadir}/gir-1.0", i.e. "/usr/share/gir-1.0" or +// "${datarootdir}", i.e. "/usr/share" or "/usr/local/share"; this is +// *not* a per-package directory, packages usually install their data +// to "${datadir}/${package_name}/". +// +// DataDirs is a [ValueSource]. +func DataDirs(pkgs ...string) (map[string]string, error) { + return ValuesOrElse("datadir", DataRootDirs, pkgs...) +} + +// GIRDirs returns a map of package-name to the directory for GObject +// Introspection Repository XML files for each requested package, or +// an error if this cannot be determined for any of the packages. The +// usual value is "${datadir}/gir-1.0", i.e. "/usr/share/gir-1.0" or // "/usr/local/share/gir-1.0". -func GIRDir(pkgs ...string) (map[string]string, error) { - return VariableOrElse("girdir", AddPathSuffix(DataDir, "gir-1.0"), pkgs...) +// +// GIRDirs is a [ValueSource]. +func GIRDirs(pkgs ...string) (map[string]string, error) { + return ValuesOrElse("girdir", AddPathSuffix(DataDirs, "gir-1.0"), pkgs...) } diff --git a/gir/pkgconfig/pkgconfig_test.go b/gir/pkgconfig/pkgconfig_test.go index ae2b67301..097811675 100644 --- a/gir/pkgconfig/pkgconfig_test.go +++ b/gir/pkgconfig/pkgconfig_test.go @@ -8,21 +8,56 @@ import ( "github.com/stretchr/testify/require" ) -func TestIncludeDirs(t *testing.T) { - type testcase struct { +func TestParseValues(t *testing.T) { + tests := map[string]struct { + inPkgs []string + inStdout string + expVals map[string]string + }{ + "missing-leading-fdo": { + []string{"gtk4", "guile-3.0", "ruby-3.0"}, + "/usr/share/guile/site/3.0 /usr/lib/ruby/site_ruby\n", + map[string]string{ + "gtk4": "", + "guile-3.0": "/usr/share/guile/site/3.0", + "ruby-3.0": "/usr/lib/ruby/site_ruby", + }, + }, + "missing-leading-pkgconf1.8": { + []string{"gtk4", "guile-3.0", "ruby-3.0"}, + " /usr/share/guile/site/3.0 /usr/lib/ruby/site_ruby\n", + map[string]string{ + "gtk4": "", + "guile-3.0": "/usr/share/guile/site/3.0", + "ruby-3.0": "/usr/lib/ruby/site_ruby", + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + out, err := parseValues(test.inPkgs, nil, test.inStdout) + require.NoError(t, err) + + assert.Equal(t, test.expVals, out) + }) + } +} + +func TestGIRDirs(t *testing.T) { + tests := []struct { inPkgs []string expGIRDirs map[string]string - } - tests := []testcase{ + }{ { - inPkgs: []string{"gtk4"}, - expGIRDirs: map[string]string{ + []string{"gtk4"}, + map[string]string{ "gtk4": "/nix/store/niw855nnjgqbq2s0iqxrk9xs5mr10rz8-gtk4-4.2.1-dev/share/gir-1.0", }, }, { - inPkgs: []string{"gtk4", "pango", "cairo", "glib-2.0", "gdk-3.0"}, - expGIRDirs: map[string]string{ + []string{"gtk4", "pango", "cairo", "glib-2.0", "gdk-3.0"}, + map[string]string{ "gtk4": "/nix/store/niw855nnjgqbq2s0iqxrk9xs5mr10rz8-gtk4-4.2.1-dev/share/gir-1.0", "pango": "/nix/store/c52730cidby7p2qwwq8cf91anqrni6lg-pango-1.48.4-dev/share/gir-1.0", "cairo": "/nix/store/gp87jysb40b919z8s7ixcilwdsiyl0rp-cairo-1.16.0-dev/share/gir-1.0", @@ -34,7 +69,7 @@ func TestIncludeDirs(t *testing.T) { for i, test := range tests { t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { - out, err := GIRDir(test.inPkgs...) + out, err := GIRDirs(test.inPkgs...) require.NoError(t, err) assert.Equal(t, test.expGIRDirs, out) ```
LukeShu commented 5 months ago

v3:

diff from v2 to v3 ```patch diff --git a/gir/gir.go b/gir/gir.go index 5e7b9f293..e6ac35bb7 100644 --- a/gir/gir.go +++ b/gir/gir.go @@ -129,6 +129,10 @@ func FindGIRFiles(pkgs ...string) ([]string, error) { // I no longer remember. for _, pkg := range pkgs { girDir := girDirs[pkg] + + // Avoid visiting the same directory twice. Sorting + + // slices.Compact(pkgs) can't save us because multiple + // pkgs might have the same girDir. if _, ok := visited[girDir]; ok { continue } diff --git a/gir/pkgconfig/pkgconfig.go b/gir/pkgconfig/pkgconfig.go index 1e15ceab7..5a6a1a0e4 100644 --- a/gir/pkgconfig/pkgconfig.go +++ b/gir/pkgconfig/pkgconfig.go @@ -15,9 +15,9 @@ import ( // A ValueSource is a function that takes a list of package names and // returns a map of package-name to some value. -type ValueSource = func(pkgs ...string) (map[string]string, error) +type ValueSource func(pkgs ...string) (map[string]string, error) -var pkgConfigVer = sync.OnceValues[string, error](func() (string, error) { +var pkgConfigVer = sync.OnceValues(func() (string, error) { var stdout strings.Builder cmd := exec.Command("pkg-config", "--version") cmd.Stdout = &stdout @@ -32,41 +32,42 @@ var pkgConfigVer = sync.OnceValues[string, error](func() (string, error) { return strings.TrimRight(stdout.String(), "\n"), nil }) -// Values returns a map of package-name to variable-value of the given -// variable name for each requested package. It is not an error for a -// variable to be unset or empty; ret[pkgname] is an empty string if -// that package does not have the variable set. -func Values(varname string, pkgs ...string) (map[string]string, error) { +// VarValues returns a map of package-name to variable-value of the +// given variable name for each requested package. It is not an error +// for a variable to be unset or empty; ret[pkgname] is an empty +// string if that package does not have the variable set. +func VarValues(varname string, pkgs ...string) (map[string]string, error) { if len(pkgs) == 0 { return nil, nil } - // There are 3 pkg-config implementations that we should work with: + // There are 2 pkg-config implementations that it would be + // nice to work with: // // 1. FDO `pkg-config` // https://www.freedesktop.org/wiki/Software/pkg-config/ // (last release was 0.29.2 in 2017) // - // 2. pkgconf 1.8.x `pkg-config` - // https://gitea.treehouse.systems/ariadne/pkgconf/src/branch/stable/1.8.x - // (last release was 1.8.1 in Jan 2023) - // - // 3. pkgconf 1.9.x/2.x `pkg-config` - // https://gitea.treehouse.systems/ariadne/pkgconf/src/branch/master + // 2. pkgconf `pkg-config` + // https://gitea.treehouse.systems/ariadne/pkgconf // (last release was 2.2.0 in Mar 2024) - - // pkgconf 1.9.0+ doesn't let us query more than one package - // at once. 1.9.0-1.9.4 do an inexplicably wrong thing if we - // ask; 1.9.5 and later ignore all packages except for the - // first one. + // + // nixpkgs is still using FDO (so FDO is the only one that we + // *need* to support), but let's be courteous to Arch Linux + // users who have moved on to pkgconf. + + // pkgconf 1.9.0+ (2022-08-07) doesn't let us query more than + // one package at once. 1.9.0-1.9.4 do an inexplicably wrong + // thing if we ask; 1.9.5 (2023-05-02) and later ignore all + // packages except for the first one. ver, err := pkgConfigVer() if err != nil { return nil, err } - if len(pkgs) > 1 && (strings.HasPrefix(ver, "1.9.") || strings.HasPrefix(ver, "2.")) { + if len(pkgs) > 1 && !strings.HasPrefix(ver, "0.") && !(strings.HasPrefix(ver, "1.") && !strings.HasPrefix(ver, "1.9.")) { ret := make(map[string]string, len(pkgs)) for _, pkg := range pkgs { - single, err := Values(varname, pkg) + single, err := VarValues(varname, pkg) if err != nil { return nil, err } @@ -142,11 +143,11 @@ func parseValues(pkgs []string, cmdline []string, stdout string) (map[string]str return ret, nil } -// ValuesOrElse is like [Values], but if a package has an empty/unset -// value, then that empty value is replaced with the value that is -// returned from the elseFn function. -func ValuesOrElse(varname string, elseFn ValueSource, pkgs ...string) (map[string]string, error) { - ret, err := Values(varname, pkgs...) +// VarValuesOrElse is like [VarValues], but if a package has an +// empty/unset value, then that empty value is replaced with the value +// that is returned from the elseFn function. +func VarValuesOrElse(varname string, elseFn ValueSource, pkgs ...string) (map[string]string, error) { + ret, err := VarValues(varname, pkgs...) if err != nil { return nil, err } @@ -193,7 +194,7 @@ func AddPathSuffix(inner ValueSource, suffix ...string) ValueSource { // // Prefixes is a [ValueSource]. func Prefixes(pkgs ...string) (map[string]string, error) { - return ValuesOrElse("prefix", func(pkgs ...string) (map[string]string, error) { + return VarValuesOrElse("prefix", func(pkgs ...string) (map[string]string, error) { return nil, fmt.Errorf("could not resolve install prefix for packages: %v", pkgs) }, pkgs...) } @@ -206,7 +207,7 @@ func Prefixes(pkgs ...string) (map[string]string, error) { // // DataRootDirs is a [ValueSource]. func DataRootDirs(pkgs ...string) (map[string]string, error) { - return ValuesOrElse("datarootdir", AddPathSuffix(Prefixes, "share"), pkgs...) + return VarValuesOrElse("datarootdir", AddPathSuffix(Prefixes, "share"), pkgs...) } // DataDirs returns a map of package-name to the base directory for @@ -219,7 +220,7 @@ func DataRootDirs(pkgs ...string) (map[string]string, error) { // // DataDirs is a [ValueSource]. func DataDirs(pkgs ...string) (map[string]string, error) { - return ValuesOrElse("datadir", DataRootDirs, pkgs...) + return VarValuesOrElse("datadir", DataRootDirs, pkgs...) } // GIRDirs returns a map of package-name to the directory for GObject @@ -230,5 +231,5 @@ func DataDirs(pkgs ...string) (map[string]string, error) { // // GIRDirs is a [ValueSource]. func GIRDirs(pkgs ...string) (map[string]string, error) { - return ValuesOrElse("girdir", AddPathSuffix(DataDirs, "gir-1.0"), pkgs...) + return VarValuesOrElse("girdir", AddPathSuffix(DataDirs, "gir-1.0"), pkgs...) } diff --git a/gir/pkgconfig/pkgconfig_test.go b/gir/pkgconfig/pkgconfig_test.go index 097811675..3cf7b8fe9 100644 --- a/gir/pkgconfig/pkgconfig_test.go +++ b/gir/pkgconfig/pkgconfig_test.go @@ -4,8 +4,7 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "github.com/alecthomas/assert/v2" ) func TestParseValues(t *testing.T) { @@ -37,7 +36,7 @@ func TestParseValues(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { out, err := parseValues(test.inPkgs, nil, test.inStdout) - require.NoError(t, err) + assert.NoError(t, err) assert.Equal(t, test.expVals, out) }) @@ -70,7 +69,7 @@ func TestGIRDirs(t *testing.T) { for i, test := range tests { t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { out, err := GIRDirs(test.inPkgs...) - require.NoError(t, err) + assert.NoError(t, err) assert.Equal(t, test.expGIRDirs, out) }) diff --git a/go.mod b/go.mod index 1e101624f..d34d40728 100644 --- a/go.mod +++ b/go.mod @@ -3,16 +3,16 @@ module github.com/diamondburned/gotk4 go 1.21.0 require ( + github.com/alecthomas/assert/v2 v2.8.1 github.com/davecgh/go-spew v1.1.1 github.com/fatih/color v1.10.0 - github.com/stretchr/testify v1.8.4 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c ) require ( + github.com/alecthomas/repr v0.4.0 // indirect + github.com/hexops/gotextdiff v1.0.3 // indirect github.com/mattn/go-colorable v0.1.8 // indirect github.com/mattn/go-isatty v0.0.12 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 571aca00c..478473e53 100644 --- a/go.sum +++ b/go.sum @@ -1,21 +1,19 @@ +github.com/alecthomas/assert/v2 v2.8.1 h1:YCxnYR6jjpfnEK5AK5SysALKdUEBPGH4Y7As6tBnDw0= +github.com/alecthomas/assert/v2 v2.8.1/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= +github.com/alecthomas/repr v0.4.0 h1:GhI2A8MACjfegCPVq9f1FLvIBS+DrQ2KQBFZP1iFzXc= +github.com/alecthomas/repr v0.4.0/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/fatih/color v1.10.0 h1:s36xzo75JdqLaaWoiEHk767eHiwo0598uUxyfiPkDsg= github.com/fatih/color v1.10.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM= +github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= +github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= github.com/mattn/go-colorable v0.1.8 h1:c1ghPdyEDarC70ftn0y+A/Ee++9zz8ljHG1b13eJ0s8= github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae h1:/WDfKMnPU+m5M4xB+6x4kaepxRw6jWvR5iDRdvjHgy8= golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= -gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/go.work.sum b/go.work.sum deleted file mode 100644 index 0a855470e..000000000 --- a/go.work.sum +++ /dev/null @@ -1 +0,0 @@ -github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= ```
LukeShu commented 5 months ago

v4:

The comment change:

diff --git a/gir/pkgconfig/pkgconfig.go b/gir/pkgconfig/pkgconfig.go
index 5a6a1a0e4..98de502cb 100644
--- a/gir/pkgconfig/pkgconfig.go
+++ b/gir/pkgconfig/pkgconfig.go
@@ -57,9 +57,12 @@ func VarValues(varname string, pkgs ...string) (map[string]string, error) {
    // users who have moved on to pkgconf.

    // pkgconf 1.9.0+ (2022-08-07) doesn't let us query more than
-   // one package at once.  1.9.0-1.9.4 do an inexplicably wrong
+   // one package at once (1.9.0-1.9.4 do an inexplicably wrong
    // thing if we ask; 1.9.5 (2023-05-02) and later ignore all
-   // packages except for the first one.
+   // packages except for the first one).  So, if we see such a
+   // version, then make a separate call for each package.  This
+   // is safe (if slow) in all cases, so we don't need to be
+   // concerned about false positives on the version number.
    ver, err := pkgConfigVer()
    if err != nil {
        return nil, err
LukeShu commented 5 months ago

v5:

LukeShu commented 5 months ago

v6:

LukeShu commented 5 months ago

v7:

diff from v6 to v7 ```patch diff --git a/gir/girgen/cmt/cmt.go b/gir/girgen/cmt/cmt.go index df94ca141..34c06939f 100644 --- a/gir/girgen/cmt/cmt.go +++ b/gir/girgen/cmt/cmt.go @@ -6,7 +6,6 @@ import ( "fmt" "go/doc" "go/doc/comment" - "go/token" "html" "log" "reflect" @@ -212,11 +211,6 @@ func GoDoc(v interface{}, indentLvl int, opts ...Option) string { func goDoc(v interface{}, indentLvl int, opts []Option) string { inf := GetInfoFields(v) - pkg, err := doc.NewFromFiles(token.NewFileSet(), nil, "TODO") - if err != nil { - panic(err) - } - var self string var orig string @@ -279,23 +273,25 @@ func goDoc(v interface{}, indentLvl int, opts []Option) string { inf.ReturnDocs) } - cmt := convertMarkdownToComment(pkg, docBuilder.String()) + cmt := convertMarkdownToComment(docBuilder.String()) if synopsize { - printer := pkg.Printer() - printer.TextWidth = -1 // don't wrap yet + printer := &comment.Printer{ + TextWidth: -1, // don't wrap yet + } cmtStr := string(printer.Text(cmt)) - cmtStr = pkg.Synopsis(cmtStr) + cmtStr = new(doc.Package).Synopsis(cmtStr) cmtStr = addPeriod(cmtStr) - cmt = pkg.Parser().Parse(cmtStr) + cmt = new(comment.Parser).Parse(cmtStr) } - printer := pkg.Printer() - // Don't use "\t" in .TextPrefix because when calculating - // .PrintWidth the printer only counts tabs as width=1. - // Instead use CommentsTabWidth spaces, and count on the final - // gofmt step to turn them into tabs. - printer.TextPrefix = strings.Repeat(" ", CommentsTabWidth*indentLvl) + "// " + printer := &comment.Printer{ + // Don't use "\t" in TextPrefix because when calculating + // .PrintWidth the printer only counts tabs as width=1. + // Instead use CommentsTabWidth spaces, and count on the final + // gofmt step to turn them into tabs. + TextPrefix: strings.Repeat(" ", CommentsTabWidth*indentLvl) + "// ", + } cmtStr := string(printer.Text(cmt)) cmtStr = transformLines(cmtStr, func(n, d int, line string) string { if line == "" && n+1 != d { @@ -455,7 +451,7 @@ func preprocessMarkdown(self, cmt string, opts []Option) string { // convertMarkdownToComment converts a (GTK-Doc-flavored / // GI-DocGen-flavored) markdown string into a parsed Go // [*comment.Doc]. -func convertMarkdownToComment(pkg *doc.Package, cmt string) *comment.Doc { +func convertMarkdownToComment(cmt string) *comment.Doc { // Fix up the codeblocks and render it using GoDoc format. codeblockFunc := func(re *regexp.Regexp, match string) string { matches := re.FindStringSubmatch(match) @@ -579,7 +575,7 @@ func convertMarkdownToComment(pkg *doc.Package, cmt string) *comment.Doc { } }) - return pkg.Parser().Parse(cmt) + return new(comment.Parser).Parse(cmt) } func transformLines(cmt string, f func(int, int, string) string) string { ```
LukeShu commented 5 months ago

v8:

Sorry for all the churn.

LukeShu commented 5 months ago

v9:

git diff -w from v8 to v9:

diff --git a/gir/girgen/cmt/cmt.go b/gir/girgen/cmt/cmt.go
index fed36cb78..dcf6242e6 100644
--- a/gir/girgen/cmt/cmt.go
+++ b/gir/girgen/cmt/cmt.go
@@ -291,6 +291,7 @@ func goDoc(v interface{}, indentLvl int, opts []Option) string {
        // Instead use CommentsTabWidth spaces, and count on the final
        // gofmt step to turn them into tabs.
        TextPrefix:     strings.Repeat(" ", CommentsTabWidth*indentLvl) + "// ",
+       TextCodePrefix: strings.Repeat(" ", CommentsTabWidth*indentLvl) + "//\t",
    }
    cmtStr := string(printer.Text(cmt))
    cmtStr = transformLines(cmtStr, func(n, d int, line string) string {
LukeShu commented 5 months ago

v10:

diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml
index a4020f971..e1afc18ac 100644
--- a/.github/workflows/qa.yml
+++ b/.github/workflows/qa.yml
@@ -36,7 +36,7 @@ jobs:

     - name: Run goimports
       run: |
-        goimports -w gir/ pkg/core/ pkg/cairo/
+        goimports -w .
         git add .
         if [[ -n "$(git status --porcelain)" ]]; then
           PAGER= git diff --cached
LukeShu commented 4 months ago

v11:

diamondburned commented 3 months ago

Sorry for the lack of update on this PR. Can you rebase this with the latest changes and re-generate the PR?

LukeShu commented 2 months ago

v12: