~skeeto/public-inbox

More explicit comment v1 PROPOSED

NRK: 1
 More explicit comment

 1 files changed, 6 insertions(+), 4 deletions(-)
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~skeeto/public-inbox/patches/38441/mbox | git am -3
Learn more about email & git

[u-config][PATCH] More explicit comment Export this patch

the current comment can be easily misunderstood as the space is easy to
miss.

it can also be misunderstood as a jab at pkgconf, when in reality it was
meant to be a proof that this edge-case isn't practical.

	[/tmp]~> cat mylib.pc
	prefix=/usr
	includedir=${prefix}/include

	Name: Mylib
	Description: my lib
	Version: 420.69.1337
	Cflags: -I ${includedir}
	[/tmp]~> pkgconf --with-path=/tmp --cflags mylib.pc
	-I /usr/include
	[/tmp]~> pkgconf --version
	1.8.0

this new comment should hopefully address any misunderstanding that
occured.
---
 u-config.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/u-config.c b/u-config.c
index f6c41e0..5a74dcf 100644
--- a/u-config.c
+++ b/u-config.c
@@ -1606,11 +1606,13 @@ static void insertsyspath(OutConfig *conf, Str path, Byte delim, Byte flag)
        Str syspath = newstr(&snapshot, prefix.len+dir.len);
        copy(copy(syspath, prefix), dir);
        // NOTE(NRK): Technically, the path doesn't need to follow the flag
        // immediately (e.g `-I /usr/include`). But I have not seen a single
        // pc file that does this.
        // immediately e.g `-I /usr/include` (notice the space between -I and
        // the include dir!).
        //
        // In fact, `pkgconf` also fails to recognize `-I /usr/include` as
        // a system include path! So this should be fine in practice.
        // But I have not seen a single pc file that does this and so we're not
        // handling this edge-case. As a proof that this should be fine in
        // practice, `pkgconf` which is used by many distros, also doesn't
        // handle it.
        if (insertstr(&snapshot, &conf->seen, syspath)) {
            *conf->arena = snapshot;
        }
-- 
2.38.1
Good thinking, thanks! Accepted and pushed.