Prep work from my GTK-Doc parser #140
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diamondburned
requested changes
Apr 10, 2024
|
v2:
diff from v1 to v2diff --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) |
diamondburned
requested changes
Apr 12, 2024
diamondburned
requested changes
Apr 12, 2024
diamondburned
requested changes
Apr 12, 2024
gir/pkgconfig/pkgconfig.go
Outdated
| if err := ensure(); err != nil { | ||
| // There are 3 pkg-config implementations that we should work with: | ||
| // | ||
| // 1. FDO `pkg-config` |
There was a problem hiding this comment.
Where are these actually used? This repository should only be concerned with whatever is on Nixpkgs (pkg-config) imo, so these shouldn't be needed. Detecting version is also not reliable, since nothing prevents these tools from being updated to the next major/minor version.
There was a problem hiding this comment.
Selfishly, I would like it to work on my Parabola (like Arch Linux) laptop without Nix. Arch hasn't shipped FDO pkg-config since 2018-05-31.
As for reliability: FDO pkg-config hasn't had an update in 7 years; jumping from 0.29 to 1.9.x or 2.x at this point seems exceedingly unlikely; and pkgconf aren't going to conflict with their own version numbers. Though pkgconf could update to 3.x--I hope to convince them to properly support querying multiple packages by then :)
There was a problem hiding this comment.
Sorry for the lack of reply on this. I'm not sure if I'm willing to support another pkg-config at the moment, and this detection is far too fragile for me to want to accept it.
There was a problem hiding this comment.
No worries. I'll drop it from the PR (might be a few days though, I'm traveling).
But, full disclosure: My main interest in the project is improving the generator as a tool for arbitrary GIR libraries, not so much with the specific GTK bindings.
This is tidying up in prep for me making real changes. None of the changes here should be functional changes.
|
v3:
diff from v2 to v3diff --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= |
|
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 |
|
v5:
|
|
v6:
|
|
v7:
diff from v6 to v7diff --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 { |
|
v8:
Sorry for all the churn. |
|
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 { |
|
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 |
The main purpose of this is to let me get other variables out of pkgconfig, which is functionality that I will use in later work.
As you can see, I got its formatting to exactly match the old formatting. This allows us to be sure that this doesn't introduce any regressions. We can make improvements to the formatting in subsequent commits.
I added these in the previous commit so that the formatting would match exactly. Now that we've verified that there are no regressions, let's drop them. This brings the formatting in-line with `gofmt`, so go ahead and have the "lint" CI job check the generated files too.
|
v11:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.