gotk4 icon indicating copy to clipboard operation
gotk4 copied to clipboard

Prep work from my GTK-Doc parser

Open LukeShu opened this issue 1 year ago • 11 comments

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 avatar Apr 10 '24 17:04 LukeShu

v2:

  • Responded to feedback
  • Fixed support for pkgconf 1.9+
  • Fixed my understanding of the FDO pkg-config "not printing spaces" behavior
diff from v1 to v2
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 avatar Apr 12 '24 06:04 LukeShu

v3:

  • re-order the commits
  • more/better comments
  • turn ValueSource from a type alias to a real type
  • drop unnecessary type parameters from the pkgConfigVer definition
  • rename Values() to VarValues()
  • flip the pkg-config version check around so that it now looks for !(version is 0.x or 1.[0-8].x) instead of version is 1.9.x or 2.x
  • use alecthomas/assert instead of stretchr/testify.
diff from v2 to v3
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 avatar Apr 12 '24 15:04 LukeShu

v4:

  • Slightly amended a comment in pkgconfig.go:
  • Added 3 more commits to the end, having gir/girgen/cmt use go/doc/comment.

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 avatar May 02 '24 22:05 LukeShu

v5:

  • rename parseComment() to convertMarkdownToComment()

LukeShu avatar May 02 '24 22:05 LukeShu

v6:

  • rename preprocessDoc() to preprocessMarkdown()
  • add doc comments to preprocessMarkdown() and convertMarkdownToComment()
  • fix a typo in a comment ("with" → "width")

LukeShu avatar May 02 '24 23:05 LukeShu

v7:

  • Minor cleanup in the penultimate commit
diff from v6 to v7
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 avatar May 03 '24 02:05 LukeShu

v8:

  • Rename convertMarkdownToComment() to convertMarkdownStringToGoDoc()

Sorry for all the churn.

LukeShu avatar May 03 '24 02:05 LukeShu

v9:

  • Fix an issue with the final formatting being slightly different than gofmt

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 avatar May 03 '24 03:05 LukeShu

v10:

  • Have the "lint" CI check be stricter, now that we can
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 avatar May 03 '24 05:05 LukeShu

v11:

  • Drop the compatibility hack for pkgconf 1.9.0+

LukeShu avatar May 08 '24 17:05 LukeShu

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

diamondburned avatar Jun 29 '24 04:06 diamondburned

v12:

  • Rebase up to 5109f19d66b75da025253527af034f53c1a72984. This turned out to be a clean rebase; no conflicts, and go generate after rebasing resulted in no changes.

LukeShu avatar Jul 08 '24 17:07 LukeShu