Re: [hackers] [st][PATCH] sgr-patch

[Quentin Rameau]

Hi Mikhail,

> Colon-separated SGR (new version; ':' is the legacy one)

Could explain a bit more what this patch is about,
with some context and telling the goal?

Re: [hackers] [st] Fix cursor move with wide glyphs || Quentin Rameau

[Quentin Rameau]

> Hi,

Hola k0ga,

> > st would always move back 1 column,
> > even with wide glyhps (using more than a single column).
> > 
> > The glyph rune is set on its first column,
> > and the other ones are to 0,
> > so loop until we detect the start of the previous glyph.  
> 
> Should this be done in every cursor addressing command?
> What happens if we position the cursor in the middle of a glyph?

That's a preliminary patch to encourage improving the matter,
I suspect there might be some unhandled side-effects in some
perticular cases, when using arbitrary cursor movement
for example, though it's unclear if those should be handled
separately or not.

Would you have a suggestion about how to handle cursor absolute
position with text position, maybe we need some addition state?

Re: [hackers] [PATCH 2/2] scripts: Fix non-portable find -perm /mode

[Quentin Rameau]

> > I guess that the original intent of the code here
> > was to to find any executable file.  
> 
> I concur. It seems to me that the default output mode for `cc` is 755
> even if the parent directory has more restrictive permissions. So the
> more concise -perm -111 should suffice. I will leave the decision up
> to Roberto though.

Well, the file creation mode for the executables is 777,
with user umask applied, so could be anything really,
like absent executable bit for others.
The install script though, once executables have been identified,
changes the target file mode to 755.

I'm not fond of that method, especially that we already know
the name of target executables.
I guess I'll propose some improvement there at some point.

Re: [hackers] [PATCH 2/2] scripts: Fix non-portable find -perm /mode

[Quentin Rameau]

> Hi Quentin,

Hola Randy,

> > -find . ! -name . -prune -type f -perm /111 |
> > +find . ! -name . -prune -type f \( -perm -u+x -o -perm -g+x -o -perm o+x 
> > \) |  
> 
> I believe `-perm -111` is equivalent and specified by POSIX.

Not exactly, -perm -111 matches if at least the mode has executable bit
for all (could be 751 for example),
while GNU -perm /111 matches if any bit is set (could be 500).

I guess that the original intent of the code here
was to to find any executable file.

[hackers] [PATCH] find: Make parameter error messages more specific

[Quentin Rameau]

Differenciate option operands from path operands
when an error is detected on the command line parameters.
---
 find.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/find.c b/find.c
index d4d7864..ef28232 100644
--- a/find.c
+++ b/find.c
@@ -809,8 +809,12 @@ parse(int argc, char **argv)
}
tok->type = op->type;
tok->u.oinfo = op;
-   } else { /* token is neither primary nor operator, must be path 
in the wrong place */
-   eprintf("paths must precede expression: %s\n", *arg);
+   } else {
+   /* token is neither primary nor operator, must be */
+   if ((*arg)[0] == '-') /* an unsupported option */
+   eprintf("unknown operand: %s\n", *arg);
+   else /* or a path in the wrong place */
+   eprintf("paths must precede expression: %s\n", 
*arg);
}
if (tok->type != LPAR && tok->type != RPAR)
ntok++; /* won't have parens in rpn */
-- 
2.42.0

[hackers] [PATCH 2/2] scripts: Fix non-portable find -perm /mode

[Quentin Rameau]

---
 scripts/mkproto | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/mkproto b/scripts/mkproto
index ab0ade4..192fe56 100755
--- a/scripts/mkproto
+++ b/scripts/mkproto
@@ -14,7 +14,7 @@ trap "rm -f scripts/proto" EXIT INT QUIT TERM
 
 (set -e
 echo d $prefix/bin $prefix/bin 755
-find . ! -name . -prune -type f -perm /111 |
+find . ! -name . -prune -type f \( -perm -u+x -o -perm -g+x -o -perm o+x \) |
 sed "s@.*@c & $prefix/bin/& 755@"
 
 echo d $manprefix/man1 $manprefix/man1 755
-- 
2.42.0

[hackers] [PATCH 1/2] scripts: Fix non-portable usage of find -maxdepth

[Quentin Rameau]

---
 scripts/mkproto | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/mkproto b/scripts/mkproto
index 0b2deb3..ab0ade4 100755
--- a/scripts/mkproto
+++ b/scripts/mkproto
@@ -14,11 +14,11 @@ trap "rm -f scripts/proto" EXIT INT QUIT TERM
 
 (set -e
 echo d $prefix/bin $prefix/bin 755
-find . -maxdepth 1 -type f -perm /111 |
+find . ! -name . -prune -type f -perm /111 |
 sed "s@.*@c & $prefix/bin/& 755@"
 
 echo d $manprefix/man1 $manprefix/man1 755
-find . -maxdepth 1 -name '*.1' |
+find . ! -name . -prune -name '*.1' |
 sed "s@.*@c & $manprefix/man1/& 644@") > $proto
 
 trap "" EXIT INT QUIT TERM
-- 
2.42.0

[hackers] [PATCH] scripts: Force file copying on install

[Quentin Rameau]

This would otherwise cause an issue using cp to copy cp to itself.
---
 scripts/install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/install b/scripts/install
index b391715..ce78c1d 100755
--- a/scripts/install
+++ b/scripts/install
@@ -9,7 +9,7 @@ do
mkdir -p $src
;;
c)
-   cp $src $dst
+   cp -f $src $dst
;;
*)
echo install: wrong entry type >&2
-- 
2.42.0

[hackers] [PATCH] make: fix rogue parameter in install target

[Quentin Rameau]

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 8da0106..95bb21d 100644
--- a/Makefile
+++ b/Makefile
@@ -219,11 +219,11 @@ getconf.h:
scripts/getconf.sh > $@
 
 install uninstall:
-   scripts/mkproto $@ $(DESTDIR)$(PREFIX) $(DESTDIR)$(MANPREFIX) proto
+   scripts/mkproto $(DESTDIR)$(PREFIX) $(DESTDIR)$(MANPREFIX) proto
scripts/$@ proto
 
 sbase-box-install: sbase-box
-   scripts/mkproto $@ $(DESTDIR)$(PREFIX) $(DESTDIR)$(MANPREFIX) proto
+   scripts/mkproto $(DESTDIR)$(PREFIX) $(DESTDIR)$(MANPREFIX) proto
scripts/$@ proto
$(DESTDIR)$(PREFIX)/bin/sbase-box -i $(DESTDIR)$(PREFIX)/bin/
 
-- 
2.42.0

Re: [hackers] [libgrapheme] Apply clang format || Laslo Hunhold

[Quentin Rameau]

> Apply clang format

Those are quite ugly

> diff --git a/src/bidirectional.c b/src/bidirectional.c
> index 6062ed6..9f1812c 100644
> --- a/src/bidirectional.c
> +++ b/src/bidirectional.c
> @@ -1400,8 +1400,8 @@ preprocess(HERODOTUS_READER *r, enum 
> grapheme_bidirectional_direction override,
>   /* store resolved paragraph level in output variable */
>   /* TODO use enum-type */
>   *resolved = (paragraph_level == 0) ?
> - GRAPHEME_BIDIRECTIONAL_DIRECTION_LTR :
> - GRAPHEME_BIDIRECTIONAL_DIRECTION_RTL;
> + GRAPHEME_BIDIRECTIONAL_DIRECTION_LTR :
> + GRAPHEME_BIDIRECTIONAL_DIRECTION_RTL;
>   }
>  
>   if (buf == NULL) {
> diff --git a/test/bidirectional.c b/test/bidirectional.c
> index 8ba0ac8..f2ba339 100644
> --- a/test/bidirectional.c
> +++ b/test/bidirectional.c
> @@ -43,8 +43,10 @@ main(int argc, char *argv[])
>   goto err;
>   }
>  
> - /* resolved paragraph level (if specified in the test) 
> */
> - if (bidirectional_test[i].resolved != 
> GRAPHEME_BIDIRECTIONAL_DIRECTION_NEUTRAL &&
> + /* resolved paragraph level (if specified in the test)
> +  */
> + if (bidirectional_test[i].resolved !=
> + GRAPHEME_BIDIRECTIONAL_DIRECTION_NEUTRAL &&
>   resolved != bidirectional_test[i].resolved) {
>   goto err;
>   }
> 

Re: [hackers] [slstatus] More LICENSE updates || drkhsh

[Quentin Rameau]

> > Would prefer just NRK :)
> > And as far as I'm aware, using pseudonames for copyright should be
> > fine.
> 
> Would you use your pseudonym on a contract to buy a house?

Would you steal a policeman's helmet and go to the toilet in it?

Re: [hackers] DWM keypad navigation?

[Quentin Rameau]

> Hello there.

Hi fossy,

> Does anyone know if there's a way to use the key-pad numbers (usually
> found on the right side of generic keyboards) to navigate tags?
> 
> I'm sure it can be done..
> The default is XK_1, for example, using TAGKEYS() macro.

The keypad keys are prefixed with XK_KP (KP like KeyPad),
and for example key number '1' is XK_KP_1

Re: [hackers] [surf][PATCH 2/2] webext: use flags for gio-unix rather than gio

[Quentin Rameau]

> Newly used g_unix_fd_* set of functions is from gio-unix. This corrects
> the flags appropriately.

I'm less inclied to applying this one though.

In any case, gpio is needed and we shouldn't replace it.

On my system, gio-unix symbols are provided by the gio library,
is that different on your system?
If so, could you provide the contents of pkg-config cflags and libs
for gio-unix-2.0?

Thanks!

Re: [hackers] [surf][PATCH 1/2] webext: add missing gio/gunixfdlist.h includes

[Quentin Rameau]

Hi Petr,

> This resolves two set of warnings pointed by compiler
> -Wimplicit-function-declaration and -Wint-conversion, where the later
> one can result with segfault caused by invalid cast from int to pointer.

Thanks.

What is strange is that on my system,
gunixfdlist.h is just included from gio/gio.h,
out of curiosity, could you give some details about your system?

The patch is correct though,
as the glib documentation specifies to include that header explicitely,
applied.

Re: [hackers] [libgrapheme] Remove superfluous printf-parameter from the example || Laslo Hunhold

[Quentin Rameau]

> Remove superfluous printf-parameter from the example
> 
> This fortunately has no functional effect, it's just redundant.

Like this sentence above. ;p

Re: [hackers] [dwm][PATCH RESEND 0/2] Const-correctness fixes

[Quentin Rameau]

> >> >I think that the cast here is actually unnecessary, isn't it?
> >>
> >> No, because FcChar8 is unsigned, but the default signedness of char may be
> >> either way.  
> >
> >And? FcNameParse will use whatever is passed
> >as a pointer to an unsigned char.  
> 
> With GCC 12.1.1 and our provided CFLAGS in config.mk:
> 
>  drw.c: In function ‘xfont_create’:
>  drw.c:122:45: warning: pointer targets in passing argument 1 of 
> ‘FcNameParse’ differ in signedness [-Wpointer-sign]
>122 | if (!(pattern = FcNameParse(fontname))) {
>| ^~~~
>| |
>| const char *

That doesn't really answer the question.

Unless what you mean is that it's necessary to shut up your compiler,
but that warning isn't an error.

Re: [hackers] [dwm][PATCH RESEND 0/2] Const-correctness fixes

[Quentin Rameau]

> I beg to differ when the code declares something const but later then decides 
> >The constness is “maintained” here, the function gets a "const FcChar8 *"  
> 
> That's quite different: the code passes it a FcChar8* which it then treats as 
> a 
> const FcChar8*. Case in point, GCC 12.1.1 and clang 14.0.6 both generate 
> different code.

I don't see why it would, and I can't reproduce it with a simple test-case.
Could you share a source/output somewhere of that?

> I really don't see why we would break the chain of const custody 
> intentionally, 
> it seems a super weird thing to push back against.

Hence my proposal to remove the cast entirely.

> >> There shouldn't be any logical change here, but it seems weird to say 
> >> things
> >> are not mutable up front and then waver about it later. Right now there's 
> >> no
> >> UB, but making sure we don't cast away the const mitigates the risk 
> >> altogether.  
> >
> >There is no risk here.  
> 
> Likely, but we're relying on system libs with unknown ABI version. Making it 
> `const` makes our intentions clear and avoids confusedly turning a const 
> variable into a non-const one. This isn't a bug fix.

The intention is to pass a fontname to FcNameParse.
Indeed, the cast makes that a bit unclear apparently,
leading to this discussion.

> >I think that the cast here is actually unnecessary, isn't it?  
> 
> No, because FcChar8 is unsigned, but the default signedness of char may be 
> either way.

And? FcNameParse will use whatever is passed
as a pointer to an unsigned char.

Re: [hackers] [dwm][PATCH RESEND 0/2] Const-correctness fixes

[Quentin Rameau]

Hi Laslo,

> I agree here. Not only should const be used to at least have a partial
> "contract" for the function parameters (C doesn't offer a lot in this
> regard and it's an easy way to prevent problems),

The contract is respected, the function will not modify the variable
pointed to by the pointer.

> it also allows the
> compiler to optimize the code better.

Could explain what kind of optimization is being prevented here?

Re: [hackers] [dwm][PATCH RESEND 0/2] Const-correctness fixes

[Quentin Rameau]

> Hi Quentin,

Hey Cris,

> In general the existing code seems confused, no?

A code isn't confused, the reader might be though.
Is there something you are not actually understanding here?

> Either we shouldn't pass them in as const in the first place,
> or we should maintain the constness that we 
> declare in the function parameters.

The constness is “maintained” here, the function gets a "const FcChar8 *"

> There shouldn't be any logical change here, but it seems weird to say things 
> are not mutable up front and then waver about it later. Right now there's no 
> UB, but making sure we don't cast away the const mitigates the risk 
> altogether.

There is no risk here.

What could be problematic is the opposite,
passing an immutable variable to a function expecting a mutable variable,
but this isn't the case here.

I think that the cast here is actually unnecessary, isn't it?

Re: [hackers] [dwm][PATCH RESEND 0/2] Const-correctness fixes

[Quentin Rameau]

Hi Cris,

> I originally sent these a few years ago and the reply was that
> const-correctness wasn't something cared about, but seeing other const
> changes in dwm, and purely const-correctness improvements in
> quark/st/libgrapheme, I figured maybe it's worth reevaluating and seeing
> if they are of interest now.

I think that the stance is still the same.

> These changes primarily avoid casting away const unnecessarily. They
> also make sure that we don't accidentally introduce non-const behaviour
> in future, and potentially UB.

Those don't “cast away constantness”, they are just cast.
While it could be argued that it makes sense to declare a variable
as const, it seems that modifying an already present cast for that
is more bikeshedding than bringing actual value.

To me, those are unuseful here, just my opinion.

Re: [hackers] [dwm][PATCH RESEND 0/2] Const-correctness fixes

[Quentin Rameau]

Hi NRK,

---
 dwm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dwm.c b/dwm.c
index 253aba7..55dd68c 100644
--- a/dwm.c
+++ b/dwm.c
@@ -1808,7 +1808,8 @@ updatebars(void)
.background_pixmap = ParentRelative,
.event_mask = ButtonPressMask|ExposureMask
};
-   XClassHint ch = {"dwm", "dwm"};
+   char s[] = "dwm";
+   XClassHint ch = { s, s };
for (m = mons; m; m = m->next) {
if (m->barwin)
continue;
-- 

I don't think that this change is necessary, strings pointed to by
XClassHint members are not modified, they're copied when necessary.

Re: [hackers] [dwm] Revert "do not call signal-unsafe function inside sighanlder" || Hiltjo Posthuma

[Quentin Rameau]

> Hi,

Hi,

> I could not find this specification in the POSIX standard, but [2] > says:
> 
> POSIX.1-2001 specifies that if the disposition of SIGCHLD
> is set to SIG_IGN or the SA_NOCLDWAIT flag is set for SIGCHLD
> (see sigaction(2)), then children that terminate do not
> become zombies and a call to wait() or waitpid() will block
> until all children have terminated, and then fail with errno
> set to ECHILD.

Maybe that's 
https://pubs.opengroup.org/onlinepubs/9699919799/functions/_Exit.html#tag_16_01_03_01

Re: [hackers] [lsw] [PATCH] Improve compliance with our coding style

[Quentin Rameau]

Hi Tom,

> - if (!XGetTextProperty(dpy, win, , netwmname) || prop.nitems == 0)
> - if (!XGetWMName(dpy, win, ) || prop.nitems == 0)
> + if (!XGetTextProperty(dpy, win, , netwmname) || !prop.nitems) {
> + if (!XGetWMName(dpy, win, ) || !prop.nitems)
>   return "";
> + }

I disagree with “!prop.nitems” change.
Variable nitems isn't a boolean value,
it's a numeral counter,
so better testing explicitly against 0.

Rest of the changes are good though, thanks. :)

Re: [hackers] [sent] [PATCH 3/3] Makefile: config.mk: Improve Makefile and config.mk

[Quentin Rameau]

> Hi Quentin,

Hi Tom o/

> > >  # includes and libs
> > > -INCS = -I. -I/usr/include -I/usr/include/freetype2 -I${X11INC}
> > > -LIBS = -L/usr/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
> > > +INCS = -I/usr/include/freetype2 -I${X11INC}
> > > +LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11
> > 
> > The -L option should go into LDFLAGS
> 
> Since $LIBS is only used to provide $LDFLAGS, we could copy it's contents
> directly into $LDFLAGS and drop $LIBS. Good catch.

No, those are two different things.

LDFLAGS are linker configuration parameters,
like indeed where to search libraries amongst other things.

The LIBS macro is to indicate what libraries to link against;
those are not configuration parameters,
but actual ordered operands in the linking chain.

> > >  # flags
> > >  CPPFLAGS = -DVERSION=\"${VERSION}\" -D_XOPEN_SOURCE=600
> > > -CFLAGS += -g -std=c99 -pedantic -Wall ${INCS} ${CPPFLAGS}
> > > -LDFLAGS += -g ${LIBS}
> > > -#CFLAGS += -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
> > > -#LDFLAGS += ${LIBS}
> > > +CFLAGS = -std=c99 -pedantic -Wall -Wstrict-prototypes 
> > > -Wold-style-definition -Os ${INCS} ${CPPFLAGS}
> > 
> > The -std option isn't necessary with the default CC.
> > I'd remove any warning from production build,
> > warnings are for development,
> > then developpers can set whatever warnings they like.
> > 
> > >  # compiler and linker
> > > -CC ?= cc
> > > +CC = cc
> > 
> > This should be removed altogether if you want a standard Makefile,
> > then CC will just be the expected c99.
> 
> I'd keep the `-std` option and the CC assignment.
> We want to be C99 compliant regardless of which compiler the user decides to 
> use.
> Additionally, CC is a configuration option in all suckless programs and
> to be consistent here seems reasonable, in my opinion.

That's the point. To be compliant with C99, use a c99 compiler,
which is the default compiler called by standard Makefiles.
By defining CC to something non-standard,
we introduce an indirection change here,
that then forces the (non-standard) -std option to be passed to random compiler
in the hope that it'll compatible with it.
If the user decides to replace the standard compiler
with another one, then they'll know which option to pass to it to make it 
respecting
the original contract.

As for some of the other suckless programs that specify CC,
that's just a legacy format that should be replaced anyway IMO.

Re: [hackers] [sent] [PATCH 3/3] Makefile: config.mk: Improve Makefile and config.mk

[Quentin Rameau]

Hi Tom,

> Let the Makefile be a bit more verbose and remove unnecessary extensions
> and flags in config.mk.
> ---
  
>  cscope: ${SRC} config.h
>   @echo cScope
> - @cscope -R -b || echo cScope not installed
> + cscope -R -b || echo cScope not installed

I think that you can also drop the || altogether,
make will just tell you whatever problem it encountered
while trying to invoque cscope.

> - @mkdir -p ${DESTDIR}${MANPREFIX}/man1
> - @cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1
> - @chmod 644 ${DESTDIR}${MANPREFIX}/man1/sent.1
> + mkdir -p ${DESTDIR}${MANPREFIX}/man1
> + cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1

I'd cp -f here too.

>  # includes and libs
> -INCS = -I. -I/usr/include -I/usr/include/freetype2 -I${X11INC}
> -LIBS = -L/usr/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
> +INCS = -I/usr/include/freetype2 -I${X11INC}
> +LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11

The -L option should go into LDFLAGS

>  # flags
>  CPPFLAGS = -DVERSION=\"${VERSION}\" -D_XOPEN_SOURCE=600
> -CFLAGS += -g -std=c99 -pedantic -Wall ${INCS} ${CPPFLAGS}
> -LDFLAGS += -g ${LIBS}
> -#CFLAGS += -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
> -#LDFLAGS += ${LIBS}
> +CFLAGS = -std=c99 -pedantic -Wall -Wstrict-prototypes -Wold-style-definition 
> -Os ${INCS} ${CPPFLAGS}

The -std option isn't necessary with the default CC.
I'd remove any warning from production build,
warnings are for development,
then developpers can set whatever warnings they like.

>  # compiler and linker
> -CC ?= cc
> +CC = cc

This should be removed altogether if you want a standard Makefile,
then CC will just be the expected c99.

Re: [hackers] [libgrapheme] Explicitly use object-files in library-generation || Laslo Hunhold

[Quentin Rameau]

Hi,
 
>  libgrapheme.a: $(SRC:=.o)
> - $(AR) rc $@ $?
> + $(AR) rc $@ $(SRC:=.o)
>   $(RANLIB) $@

This works as intended with $?, because then you only update objects
that are out of date, not *all* objects inconditionally (just note that
you might want the -u flag too).

Re: [hackers] [dwm][PATCH] preserve clients on old tags when renewing dwm

[Quentin Rameau]

Hey Arda,

> > I am really sorry, this is my first time posting a patch. I thought this
> > list was for patches and mainstream both. I wanted to send this as a patch,
> > where should I redirect this?
>
> I will redirect it to Wiki again, I have misread the website. Sorry for the
> inconvenience.

No problem at all, you send it to the correct list.
You send a patch, people discuss it, it's only me giving one opinion.

We can wait for other people to chime in, see what they think,
that's how it works :)

Re: [hackers] [dwm][PATCH] preserve clients on old tags when renewing dwm

[Quentin Rameau]

Hi Arda,

I like the idea, thanks for the work.
I'm not sure it really should be merged into the main repository though,
a Window Manager doesn't have the purpose of being restarted on the fly,
besides for debugging/testing purposes.

Maybe it would have a better place in the patches section?

Re: [hackers] [sbase][PATCH] tar: support extracting long paths, link targets, and times.

[Quentin Rameau]

Hi Andrew,

> Posix tarballs use extended headers to represent paths and values that do
> not fit in the original ustar header format. This patch implements parsing
> and handling of a subset of these extended headers.  The motivating
> tarball was the gcc source code, which exceeds the original path limit.

Thanks for the patches, they look good to me be I'll do a proper review
this weekend if nobody else did before.

Cheers!

Re: [hackers] [dwm|dmenu|st][PATCH] strip the installed binary

[Quentin Rameau]

Hi Hiltjo,

> I don't like this.
> 
> I'd rather have it so the Makefile respects the system or package system 
> CFLAGS
> and LDFLAGS by default.  Then someone can do: make CFLAGS="-Os" LDFLAGS="-s"
> etc.
> 
> LDFLAGS="-s" is practically the same as calling strip and stripping it.
> 
> It is up to the distro package/ports maintainer to strip symbols (or not).
> This can be an additional packaging step.

Allright, that's valid to me (don't thank me for my meaningful addition
to this conversation).

> As a off-topic side-note I think we removing config.mk and just having the
> Makefile is simpler too.

I think that config.mk makes a bit more obvious (maybe)
what is supposed to be configured,
and what is part of the “stock” Makefile rules.

This could also be a commented section within the Makefile though.

Re: [hackers] [dwm|dmenu|st][PATCH] strip the installed binary

[Quentin Rameau]

> Hi,

Hi NRK,

> Attached patches to strip the binary on installation for dwm, dmenu and
> st.

Looks good to me, thanks!

Re: [hackers] [dmenu] fix incorrect comment, math is hard || Hiltjo Posthuma

[Quentin Rameau]

Hi Hiljto,

> fix incorrect comment, math is hard

diff --git a/dmenu.c b/dmenu.c
index 571bc35..3595267 100644
--- a/dmenu.c
+++ b/dmenu.c
@@ -673,7 +673,7 @@ setup(void)
mw = wa.width;
}
promptw = (prompt && *prompt) ? TEXTW(prompt) - lrpad / 4 : 0;
-   inputw = mw / 3; /* input width: ~33% of monitor width */
+   inputw = mw / 3; /* input width: ~33.333% of monitor width */
match();
 
/* create menu window */

Re: [hackers] [libsl|dmenu][PATCH v2] Fix truncation issues and improve performance

[Quentin Rameau]

> On Mon, Mar 28, 2022 at 05:57:48PM +0600, NRK wrote:
> > And on the topic of ellipsis_width, we currently don't account for
> > fallback font:
> > 
> > usedfont = drw->fonts;
> > drw_font_getexts(usedfont, "...", 3, _width, NULL);
> > 
> > The assumption here was that every font should have '.' but if that
> > turns out to be wrong, then the font width will be incorrect.

I think that we can have as a requirement that the system provides a
font with the glyph for dot.

> Attached a patch fixing this issue, as well as a patch guarding against
> calling drw_text() with 0 width.
> 
> Think both these issues should be extremely rare, but can't hurt dealing
> with them anyways.
> 
> - NRK

> @@ -283,7 +284,10 @@ drw_text(Drw *drw, int x, int y, unsigned int w, 
> unsigned int h, unsigned int lp
>   }
>  
>   usedfont = drw->fonts;
> - drw_font_getexts(usedfont, "...", 3, _width, NULL);
> + if (ellipsis_width < 0) {
> + ellipsis_width = 0; /* stop infinite recursion */
> + ellipsis_width = drw_fontset_getwidth(drw, "...");

I don't understand how setting it twice in a row stops a recursion.
Will not the drw_fontset_getwidth() call *always* overwrite any value set 
before?

> + }
>   while (1) {
>   ew = ellipsis_len = utf8strlen = 0;
>   utf8str = text;

Re: [hackers] [libsl|dmenu][PATCH v2] Fix truncation issues and improve performance

[Quentin Rameau]

> Hi,

Hi NRK,

> Fixed a couple bugs/issues with the previous patches and attached the
> amended ones. I'm confident that these patches shouldn't have any
> remaining logic issues, but feel free to point them out in case there is.

> Additionally, there's one *new* patch attached, which fixes the
> performance issue in case we have no font match for a given codepoint.

I haven't reviewed those (yet), but thanks for your work on this!

Re: [hackers] [sbase][PATCH] flock: call exec without forking

[Quentin Rameau]

> Don't believe the '-F' flag is portable so avoided trying to add
> it.

flock(1) itself isn't portable, so none of its flag are.

Re: [hackers] [st][PATCH] Fix truecolor being slightly inaccurate

[Quentin Rameau]

Hi Yutao,

> The displayed color in truecolor mode is sometimes darker than
> requested. The inaccuracy can be verified with the following example,
> which should paint a white block but instead produces color #fefefe.
> 
> printf '\e[38;2;255;255;255m\u2588\u2588\u2588\n'

I get #ff on both vanilla st and with your patch applied.

Re: [hackers] st][PATCH - proper escape sequence for CTRL+HOME

[Quentin Rameau]

Hi again Dave,

> > Let that be a poor lesson for everybody, to you that coming to
> > unfamiliar people with a negative attitude usually draws negative
> > attitude in response.  
> 
> Nothing was wrong with my comment. You just chose to interpret it poorly--for 
> selfish reasons.

Allright, but then there's no need to continue ranting on the list,
please just the nobler one here and go on with life.

Re: [hackers] st][PATCH - proper escape sequence for CTRL+HOME

[Quentin Rameau]

Hi Dave,

> It's not the language that's the problem, it's the piss-poor attitude.
> 
> At first, I thought his response might have been a fluke. But then after 
> reading the other extremely rude comments made in response to mine, it seems 
> this is a group of extremely toxic people who are absolutely uninterested in 
> receiving any kind of constructive criticism whatsoever.
> 
> I'll see myself out.

Let that be a poor lesson for everybody, to you that coming to
unfamiliar people with a negative attitude usually draws negative
attitude in response.
To others, that it isn't usually less constructive to just ignore such
bad attitude in the first and wait for a potential change in the
approach before answering anything.

Re: [hackers] [dwm][PATCH] Added support for RTL languages (Farsi, Arabic and Hebrew using the FriBiDi library)

[Quentin Rameau]

Hi Mahdi,

> hello sir, thank you I wanted this to be in the wiki, should I write a wiki 
> for the patch and send it over as markdown? or someone from the team does it 
> themselves?

The wiki is open, you can push yourself your changes.

Information can be found here: http://suckless.org/wiki/

The idea basically is to create a new directory with an index.md
explaining your patch and providing a link to it (that you would upload
alongside it).

You can look at how other patches are published there.

Thanks for your interest!

Re: [hackers] [dmenu] revert using strcasestr and use a more optimized portable version || Hiltjo Posthuma

[Quentin Rameau]

> revert using strcasestr and use a more optimized portable version
> 
> ... compared to the old cistrstr().

I confirm that it's an improvement on my musl/Linux to both the
old master code and reverted patch.

Thanks Hiltjo!

Re: [hackers] [dmenu] follow-up fix: add -D_GNU_SOURCE for strcasestr for some systems || Hiltjo Posthuma

[Quentin Rameau]

Hola,

Here are my two cents,

> On Mon, Feb 07, 2022 at 10:41:58AM +0100, Laslo Hunhold wrote:
> > On Mon, 7 Feb 2022 10:36:46 +0100 (CET)
> > g...@suckless.org wrote:
> > 
> > Dear Hiltjo,
> > 
> > > follow-up fix: add -D_GNU_SOURCE for strcasestr for some systems
> > 
> > wouldn't it be better to avoid GNU-extensions in code?
> > 
> > With best regards
> > 
> > Laslo
> > 
> 
> I kindof expected a reply like this. In general I don't disagree.
> 
> This function is available on many systems for decades.
> 
> On some systems like OpenBSD the -D_GNU_SOURCE is not needed.

I still think it's kind of problematic to add a non-portable function
here.

What we do in other places is to provide a fallback implementation of
such functions, so maybe we should do that here too.

Also, without questioning your actual push, Hiltjo, maybe that's not the
best way to fix the performance issue and we could invest more time to
think about this.

Re: [hackers] [libgrapheme] Refactor Makefile, add dist-target and add test-util || Laslo Hunhold

[Quentin Rameau]

Hi Laslo,

As a note,

> -GEN = gen/grapheme gen/grapheme-test
> -LIB = src/grapheme src/utf8 src/util
> -TEST = test/grapheme test/grapheme-performance test/utf8-decode 
> test/utf8-encode
> -
> -MAN3 = man/lg_grapheme_isbreak.3 man/lg_grapheme_nextbreak.3
> +GEN =\
> + gen/grapheme\
> + gen/grapheme-test
> +SRC =\
> + src/grapheme\
> + src/utf8\
> + src/util
> +TEST =\
> + test/grapheme\
> + test/grapheme-performance\
> + test/utf8-decode\
> + test/utf8-encode
> +MAN3 =\
> + man/lg_grapheme_isbreak.3\
> + man/lg_grapheme_nextbreak.3
>  MAN7 = man/libgrapheme.7
>  
>  all: libgrapheme.a libgrapheme.so

The idiomatic way of using those is to escape the newline on every macro
line.
The goal here is to help producing less noise in patches which add or
remove lines there, so that only the actual concerned lines are
modified, not the one that may be the last because you now need to add
or remove a '\' there.

Re: [hackers][st][PATCH] Add support for OSC color sequences.

[Quentin Rameau]

Hi Raheman,

Just from a coding standpoint,

> diff --git a/config.def.h b/config.def.h
> index 6f05dce..91ab8ca 100644
> --- a/config.def.h
> +++ b/config.def.h
> @@ -120,6 +120,8 @@ static const char *colorname[] = {
>   /* more colors can be added after 255 to use with DefaultXX */
>   "#cc",
>   "#55",
> + "gray90", /* default foreground colour */
> + "black", /* default background colour */
>  };
>  
>  
> @@ -127,9 +129,9 @@ static const char *colorname[] = {
>   * Default colors (colorname index)
>   * foreground, background, cursor, reverse cursor
>   */
> -unsigned int defaultfg = 7;
> -unsigned int defaultbg = 0;
> -static unsigned int defaultcs = 256;
> +unsigned int defaultfg = 258;
> +unsigned int defaultbg = 259;
> +unsigned int defaultcs = 256;
>  static unsigned int defaultrcs = 257;
>  
>  /*
> diff --git a/st.c b/st.c
> index a9338e1..ad5c8e8 100644
> --- a/st.c
> +++ b/st.c
> @@ -1842,6 +1842,42 @@ csireset(void)
>   memset(, 0, sizeof(csiescseq));
>  }
>  
> +void
> +osc4_color_response(int num)
> +{
> + int n;
> + char buf[32];
> + unsigned char r,g,b;
> +
> + if (xgetcolor(num, ,,)) {
> + fprintf(stderr, "erresc: failed to fetch osc4 color %d\n", num);
> + return;
> + }
> +
> + n = snprintf(buf, sizeof buf, 
> "\033]4;%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
> +  num, r, r, g, g, b, b);
> +
> + ttywrite(buf, n, 1);
> +}
> +
> +void
> +osc_color_response(int index, int num)
> +{
> + int n;
> + char buf[32];
> + unsigned char r,g,b;
> +
> + if (xgetcolor(index, ,,)) {
> + fprintf(stderr, "erresc: failed to fetch osc color %d\n", 
> index);
> + return;
> + }
> +
> + n = snprintf(buf, sizeof buf, 
> "\033]%d;rgb:%02x%02x/%02x%02x/%02x%02x\007", 
> +  num, r, r, g, g, b, b);
> +
> + ttywrite(buf, n, 1);
> +}
> +
>  void
>  strhandle(void)
>  {

It seems that both osc4_color_response and osc_color response are the exact
same function, just the print format string differs.
This could be factorized with only a parameter for switching the string.

> @@ -1880,6 +1916,45 @@ strhandle(void)
>   }
>   }
>   return;
> + case 10:
> + if (narg < 2)
> + break;
> +
> + p = strescseq.args[1];
> +
> + if (!strcmp(p, "?"))
> + osc_color_response(defaultfg, 10);
> + else if (xsetcolorname(defaultfg, p))
> + fprintf(stderr, "erresc: invalid foreground 
> color: %s\n", p);
> + else
> + redraw();
> + break;
> + case 11:
> + if (narg < 2)
> + break;
> +
> + p = strescseq.args[1];
> +
> + if (!strcmp(p, "?"))
> + osc_color_response(defaultbg, 11);
> + else if (xsetcolorname(defaultbg, p))
> + fprintf(stderr, "erresc: invalid background 
> color: %s\n", p);
> + else
> + redraw();
> + break;
> + case 12:
> + if (narg < 2)
> + break;
> +
> + p = strescseq.args[1];
> +
> + if (!strcmp(p, "?"))
> + osc_color_response(defaultcs, 12);
> + else if (xsetcolorname(defaultcs, p))
> + fprintf(stderr, "erresc: invalid cursor color: 
> %s\n", p);
> + else
> + redraw();
> + break;
>   case 4: /* color set */
>   if (narg < 3)
>   break;

Again, case 10, 11, and 12, looks to be the exact same code, only the
parameter passed to osc_color_response differ, but you already have
that variable at hand.
Those three cases could be joined in a fallthrough that would call a
function with that code just differing depending on the parameter value.

> @@ -1887,7 +1962,10 @@ strhandle(void)
>   /* FALLTHROUGH */
>   case 104: /* color reset, here p = NULL */
>   j = (narg > 1) ? atoi(strescseq.args[1]) : -1;
> - if (xsetcolorname(j, p)) {
> +
> + if (!strcmp(p, "?"))
> + osc4_color_response(j);
> + else if (xsetcolorname(j, p)) {
>   if (par == 104 && narg <= 1)
>   return; /* color reset without 
> parameter */
>   fprintf(stderr, "erresc: invalid color j=%d, 
> p=%s\n",
> diff --git a/st.h b/st.h
> 

Re: [hackers] [sbase][PATCH] nohup: Open nohup.out WRONLY

[Quentin Rameau]

Hi Arthur,

> Open nohup.out write-only instead of not specifying the access permissions
> instead of getting undefined behavior (which probably results in stdout
> not being writable).
> 
> - if ((fd = open("nohup.out", O_APPEND | O_CREAT, S_IRUSR | 
> S_IWUSR)) < 0)
> + if ((fd = open("nohup.out", O_WRONLY | O_APPEND | O_CREAT, 
> S_IRUSR | S_IWUSR)) < 0)

Thanks for the patch, it's valid and Michael should merge it when he's
got time.

Re: [hackers] [surf][patch] Use signed chars for webext messages.

[Quentin Rameau]

Hello Noah,

> Plain chars are currently used for messaging, but sometimes negative values 
> are sent, e.g. in scrollv. on x86 this doesn't cause a problem but chars are 
> unsigned on my arm computer.

Thanks, I'll apply that soon.

Re: [hackers] [PATCH] Add a configuration option for fullscreen locking

[Quentin Rameau]

> Hey folks,

Hello Cris,

> Looking at the conversation, it seems like the concern is about interaction 
> with the fakefullscreen[0] patch, if I understand correctly.

No it's not.

[hackers] [PATCH] Add a configuration option for fullscreen locking

[Quentin Rameau]

Some people are annoyed to have this new behaviour forced for some
application which use fake fullscreen.
---
 config.def.h | 1 +
 dwm.c| 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/config.def.h b/config.def.h
index 1c0b587..a2ac963 100644
--- a/config.def.h
+++ b/config.def.h
@@ -35,6 +35,7 @@ static const Rule rules[] = {
 static const float mfact = 0.55; /* factor of master area size 
[0.05..0.95] */
 static const int nmaster = 1;/* number of clients in master area */
 static const int resizehints = 1;/* 1 means respect size hints in tiled 
resizals */
+static const int lockfullscreen = 1; /* 1 will force focus on the fullscreen 
window */
 
 static const Layout layouts[] = {
/* symbol arrange function */
diff --git a/dwm.c b/dwm.c
index b0b3466..5b41d4c 100644
--- a/dwm.c
+++ b/dwm.c
@@ -835,7 +835,7 @@ focusstack(const Arg *arg)
 {
Client *c = NULL, *i;
 
-   if (!selmon->sel || selmon->sel->isfullscreen)
+   if (!selmon->sel || selmon->sel->isfullscreen && lockfullscreen)
return;
if (arg->i > 0) {
for (c = selmon->sel->next; c && !ISVISIBLE(c); c = c->next);
-- 
2.32.0

Re: [hackers] dvtm: add a comma at the end of kf22 entry in dvtm.info

[Quentin Rameau]

Hi Greg,

> > I see.  I wrote to this list because of these instructions from 
> > https://github.com/martanne/dvtm:  "If you have comments, suggestions, 
> > ideas, a bug report, a patch or something else related to dvtm then write 
> > to the suckless developer mailing list or contact me directly."
> > 
> > Perhaps my message would have been better sent to dev than hackers?
> 
> Don't send it to the suckless mailinglist.

Na, dvtm is not on suckless dev anymore, you can contact Marc directly
for the patch, you can find an email address contact in the commits!

Thanks :)

Re: [hackers] [quark] http: fix default index serving

[Quentin Rameau]

> Dear Quentin,
>
> I took a look at the archives[0] and have merged it in [1], however,
> changed it into an else-case (to make it not depend on the fallthroughs
> in the if only) and changed the mime-check so the mime-type is matched
> against the docindex-path.

I would prefer that you keep rightful authors of patches instead of
changing the style a bit and committing in your own name.
This isn't respectful of contributors and seems to be a recurring issue
with you.

If you want to change the style, you can discuss it with the authors,
and amend the commit before pushing instead of doing that.

I would prefer that you revert the commit, and do it properly (which
would be good as it would also be explained in the development history).

> The http_prepare_response()-function is pretty messy, especially in
> regard to stale data, which this bug is also based on. I'm working on
> making it more resilient by splitting the discrete sub-problems into
> separate functions.

Yes, but that's also partly due to the style, these are no
“fallthrough” cases, there are early returns, and it's easier to read
them as such instead of putting them into if-then-else blocks
everywhere.

> Thanks for finding this issue and your patch! I must admit that this
> issue slipped past me because I only checked quark's behaviour with
> "curl -I", which effectively masked this problem.

Yeah, but it clearly shows an unexpected server-side early connection
close otherwise, which is what led me to find the problem!

> With best regards

Next time

Re: [hackers] [quark] http: fix default index serving

[Quentin Rameau]

bump

[hackers] [quark] http: fix default index serving

[Quentin Rameau]

The previous code would find and stat the default index file,
but would not append it to the file served, resulting in
giving back a 0-length content but with a Content-Length
header of the size of the index.
---
 http.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/http.c b/http.c
index dc32290..96dd540 100644
--- a/http.c
+++ b/http.c
@@ -761,6 +761,11 @@ http_prepare_response(const struct request *req, struct 
response *res,
goto err;
}
}
+   /* copy the found index back to the final path */
+   if (esnprintf(res->path, sizeof(res->path), "%s", tmpuri)) {
+   s = S_REQUEST_TOO_LARGE;
+   goto err;
+   }
}
 
/* modified since */
-- 
2.28.0

Re: [hackers] [tabbed][PATCH] Remove quotes around variables from commands

[Quentin Rameau]

> From: Sebastian LaVine 

Hi Sebastian,

> Previously, if you had changed your PREFIX in config.mk to something
> like ~/.local, then instead of installing to /home/user/.local, a
> directory called '~' would be created in the project directory. Commands
> are also performed without quotes in the Makefiles of other suckless
> projects, like st or dwm.

That means that the user quoted the PREFIX in the first place, so
that's how it's wanted to be used.

The calling shell would do the tilde expansion (if the user wishes so),
the Makefile command itself shouldn't here.

e.g.:

$ make PREFIX='~/.local' install
will install to the quoted '~/.local' directory.

$ make PREFIX=~/.local install
will install to the expanded /home/$USER/.local directory.

Re: [hackers] [dmenu][PATCH 0/2] New option for floating window

[Quentin Rameau]

> Regarding the second one, I think you could put it on the wiki.

Oh and I forgot, you should test in on displays with several monitors
if you can!

Re: [hackers] [dmenu][PATCH 0/2] New option for floating window

[Quentin Rameau]

Hello Jérémy,

Thank you for you patches.

> Those 2 patches allows to have dmenu displayed in a pop-up window in the
> middle of the screen. Personally I like the way it looks and I think
> that this option (-fw) could be available for everyone.

I'll comment both here as these are general remasks.

Regarding the first one, isn't that already the current behaviour?

$ printf '%s\n' foo bar bar|./dmenu -l 10

Only displays 3 lines, not 10.
Could you give a use-case?

That aside, try to follow the coding style of the rest of the code,
here don't put underscored variables like item_count, you can just use
nitem for example, or something shorter.

Regarding the second one, I think you could put it on the wiki.

But again, try to follow the style, -option is the worst of both
dash-options worlds, use -o options only, that should be enough.
And if your tool really have more options than you can count in the
ascii set, either remove some, or at worse use --option, but not
-option.

> This is my first contribution to this project, please let me know if
> everything is ok with my patches.

Thanks again, keep on working and improving!

Re: [hackers] [quark] Prevent overflow in strtonum()-parameters || Laslo Hunhold

[Quentin Rameau]

> Make sure not to overflow the long long value. Given the standard
> doesn't bring any tangible guarantees for the upper limits of size_t,

SIZE_MAX is the tangible guarantee for the upper limit of size_t.

Re: [hackers] [dmenu][PATCH] Vim-like vertical and horizontal keybindings

[Quentin Rameau]

> From: Spenser Truex 

Hello Spenser,

Thanks for the patch, a few comments :

Throughout the patch, there are indentation issues.

> Ensure that the keybindings are like vim, and not just made up.

This should be rephrased, precising that it's about vertical
mode, and nothing is made up.

> Old behaviour:
> Alt-h,l moves one element
> Alt-j,k moves one page
> 
> New behaviour:
> Alt-h,l moves left/right or pages on a vertical listing
> Alt-j,k moves up/down on a vertical listing or pages a horizontal one
> 
> This new behaviour maintains the heuristics for these keys:
> h is LEFT
> l is RIGHT
> j is DOWN
> k is UP
> ---
>  config.def.h |  5 -
>  dmenu.c  | 37 +++--
>  2 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/config.def.h b/config.def.h
> index 1edb647..0d7748d 100644
> --- a/config.def.h
> +++ b/config.def.h
> @@ -13,8 +13,11 @@ static const char *colors[SchemeLast][2] = {
>   [SchemeSel] = { "#ee", "#005577" },
>   [SchemeOut] = { "#00", "#00" },
>  };
> -/* -l option; if nonzero, dmenu uses vertical list with given number of 
> lines */
> +/* -l option sets both of these to true */
> +/* if nonzero, dmenu uses given number of lines */
>  static unsigned int lines  = 0;
> +/* If nonzero, show vertical instead of a horizontal listing */
> +static unsigned int vertical   = 0;

Is it really useful to have an option introducing different behaviour?
You could just keep the lines > 0 check.
 
>  /*
>   * Characters not considered part of a word while deleting words
> diff --git a/dmenu.c b/dmenu.c
> index 65f25ce..cb28913 100644
> --- a/dmenu.c
> +++ b/dmenu.c
> @@ -76,16 +76,16 @@ calcoffsets(void)
>  {
>   int i, n;
> 
> - if (lines > 0)
> + if (vertical)
>   n = lines * bh;
>   else
>   n = mw - (promptw + inputw + TEXTW("<") + TEXTW(">"));
>   /* calculate which items will begin the next page and previous page */
>   for (i = 0, next = curr; next; next = next->right)
> - if ((i += (lines > 0) ? bh : MIN(TEXTW(next->text), n)) > n)
> + if ((i += vertical ? bh : MIN(TEXTW(next->text), n)) > n)
>   break;
>   for (i = 0, prev = curr; prev && prev->left; prev = prev->left)
> - if ((i += (lines > 0) ? bh : MIN(TEXTW(prev->left->text), n)) > 
> n)
> + if ((i += vertical ? bh : MIN(TEXTW(prev->left->text), n)) > n)
>   break;
>  }
> 
> @@ -141,7 +141,7 @@ drawmenu(void)
>   x = drw_text(drw, x, 0, promptw, bh, lrpad / 2, prompt, 0);
>   }
>   /* draw input field */
> - w = (lines > 0 || !matches) ? mw - x : inputw;
> + w = (vertical || !matches) ? mw - x : inputw;
>   drw_setscheme(drw, scheme[SchemeNorm]);
>   drw_text(drw, x, 0, w, bh, lrpad / 2, text, 0);
> 
> @@ -151,7 +151,7 @@ drawmenu(void)
>   drw_rect(drw, x + curpos, 2, 2, bh - 4, 1, 0);
>   }
> 
> - if (lines > 0) {
> + if (vertical) {
>   /* draw vertical list */
>   for (item = curr; item != next; item = item->right)
>   drawitem(item, x, y += bh, mw - x);
> @@ -384,10 +384,18 @@ keypress(XKeyEvent *ev)
>   goto draw;
>   case XK_g: ksym = XK_Home;  break;
>   case XK_G: ksym = XK_End;   break;
> - case XK_h: ksym = XK_Up;break;
> - case XK_j: ksym = XK_Next;  break;
> - case XK_k: ksym = XK_Prior; break;
> - case XK_l: ksym = XK_Down;  break;
> + case XK_j:
> +vertical ? ksym = XK_Down : (ksym = XK_Next);

I think it would better to keep something like:

case XK_j: ksym = vertical ? XK_Down : XK_Next; break;

With eventually the break on a new line.

Same for those other ones.

> +break;
> + case XK_k:
> +vertical ? ksym = XK_Up : (ksym = XK_Prior);
> +break;
> +case XK_h:
> +vertical ? ksym = XK_Prior : (ksym = XK_Up);
> +break;
> + case XK_l:
> +vertical ? ksym = XK_Next : (ksym = XK_Down);
> +break;
>   default:
>   return;
>   }
> @@ -437,11 +445,11 @@ insert:
>   calcoffsets();
>   break;
>   case XK_Left:
> - if (cursor > 0 && (!sel || !sel->left || lines > 0)) {
> + if (cursor > 0 && (!sel || !sel->left || vertical)) {
>   cursor = nextrune(-1);
>   break;
>   }
> - if (lines > 0)
> + if (vertical)
>   return;
>   /* fallthrough */
>   case XK_Up:
> @@ -477,7 +485,7 @@ insert:
>   cursor = nextrune(+1);
>   break;
>   }
> - if (lines > 0)
> + if (vertical)
>   

Re: [hackers] [dmenu] bump version to 5.0 || Hiltjo Posthuma

[Quentin Rameau]

> bump version to 5.0

\o/

Re: [hackers] [svkbd] Update Makefile

[Quentin Rameau]

Hello Job,

> 'make clean' now removes config.h

Wrong

> 'make dist' should no longer exit with error code 1

Good

Re: [hackers] [sbase] install: Use fchown to change owner || Michael Forney

[Quentin Rameau]

> >> +  if (fchown(f2, owner, group) < 0)
> >> +  eprintf("lchown %s:", s2);
> >
> > Little typo here ^
> 
> Whoops, missed that. In the follow up commit, I used the correct
> function name, so I think it's just a mistake from when I split my
> changes into two commits.

Oh right, I missed the change in the other commit because it arrived
before this one on the mailing list, forget about my comment then!

Re: [hackers] [sbase] install: Use fchown to change owner || Michael Forney

[Quentin Rameau]

Hi Michael,

> + if (fchown(f2, owner, group) < 0)
> + eprintf("lchown %s:", s2);

Little typo here ^

Re: [hackers] [PATCH] tar: if first argument doesn't have a leading dash, prepend one

[Quentin Rameau]

> > At least, I would print a warning if the old style syntax is seen so
> > people start fixing their scripts.  
> 
> On what basis are scripts written to the SUSv2 specification broken?

On the basis that tar was already specified as deprecated in SUSv1 26
years ago, and that sbase follows last POSIX specification (3 more
releases since).

> Because we said so? Ethan looked at a bunch of tar implementations,
> and some did not support the hyphenated options. Therefore, the most
> portable way to call tar is with the old-style options.

Again, sbase goal is not to support all deprecated interfaces, or
extentsions, you can see commonly see around.

> Personally, I think tar is a hopeless interface and we should
> implement pax in sbase. I started on an implementation a while ago,
> but it is unfinished. After this, we can remove this tar
> implementation, which has some known bugs and deficiencies, and
> possibly replace it with a tar compatibility interface to pax. This
> tar compatibility interface should probably support the hyphen-less
> option key, since its whole purpose is legacy compatibility.

We don't need to have 100% coverage finished tools in sbase, maybe push
what you already have for pax so that people can help the effort, that's
also what sbase is about.
Then we can see if we move tar outside the repo, if people want to keep
using this implementation.

Ok to push the patch for historical reasons, but I don't think we
should encourage this kind of additions.

Re: [hackers] [PATCH] tar: if first argument doesn't have a leading dash, prepend one

[Quentin Rameau]

> Every other tar implementation I've looked at supports either both
> dash-less argument, and arguments with dashes, or only dash-less
> argument. sbase tar is the only tar that only supports arguments with a
> dash, so to use tar as portably as possible, one has to not have a dash
> with the argument, and sbase tar should be made to accept that style of
> argument to fit with the more portable usage.

Yes, the more portable usage being the one specified buy the standard
utility guidelines as it is specified and supported by every
implementations.

Re: [hackers] [PATCH] tar: if first argument doesn't have a leading dash, prepend one

[Quentin Rameau]

Hi all,

> Dear Michael,
> 
> > Thanks for the patch, Ethan.
> > 
> > This patch looks fine to me, but seeing as we used to support this
> > usage before it was reverted in [0], I'd like to see if anyone else
> > has comments. The approach you used avoids the code duplication that
> > may have been the motivation for its removal.  
> 
> I have objections for this patch. I don't like the old style and it's
> horribly annoying with tools like unrar and 7z as well. I don't know
> why the compression tools all ignore years of established command line
> syntax, but I think we shouldn't chime in to this mad play.

I agree too we shouldn't pass that in.

One purpose of writing sbase is to guide people into using tools in a
“POSIX-ly” correct manner.
If we just let in all syntax around then it defeats that purpose.

Re: [hackers] [scroll] Remove ^E and ^Y || Roberto E. Vargas

[Quentin Rameau]

> - {"\031",SCROLL_UP,1},   /* mouse wheel up */
> - {"\005",SCROLL_DOWN,  1},   /* mouse wheel Down */
> + //{"\031",SCROLL_UP,1},   /* mouse wheel up */
> + //{"\005",SCROLL_DOWN,  1},   /* mouse wheel Down */

Did you forget to remove the two lines?

If this was intentional, I think a proper comment about those would be
better, otherwise it just looks like forgotten test code, with no
indication as why it's here and what it does.

Re: [hackers] [dwm][patch] Fix transparent borders

[Quentin Rameau]

> Hi,

Hi Jakub,

> Commit: 
> 

Please simply attach patches to email directly instead of redirecting
to third-party prodivers/protocols.

Re: [hackers] [scroll] Dereference pointers || Jochen Sprickerhof

[Quentin Rameau]

Hi Jochen,

> Dereference pointers

> - if (x <= 0 || y <= 0)
> + if (*x <= 0 || *y <= 0)
>   die("invalid cursor position: x=%d y=%d", x, y);
   /--^--^
Here too :)

Re: [hackers] [scroll] Catch invalid cursor positions || Jochen Sprickerhof

[Quentin Rameau]

> gcc emits this warning when using the "-Wextra" option.  Should x and y be
> dereferenced?

yes

Re: [hackers] [scroll] Fix make on OpenBSD || Jochen Sprickerhof

[Quentin Rameau]

> Sure, provide a patch to OpenBSD ;).

Glad that you agree.
The point is that this is not the correct place to fix this issue.

> Does it break anything this way?

It might break reader's understanding about how make works.

Re: [hackers] [scroll] Fix make on OpenBSD || Jochen Sprickerhof

[Quentin Rameau]

> Fix make on OpenBSD
>
> -scroll: config.h
> +scroll: scroll.c config.h

I think it'd be better to fix OpenBSD make or let OpenBSD port
maintainers to patch the software rather than pushing changes do
standard code because of some buggy implementations.

Re: [hackers] [scroll] Rework the Makefile a bit || Quentin Rameau

[Quentin Rameau]

Hey Jochen,

Thanks for applying!

[hackers] [scroll][PATCH 1/2] Rework the Makefile a bit

[Quentin Rameau]

---
 Makefile  | 18 --
 config.mk | 13 ++---
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 4677e7a..c3e1c50 100644
--- a/Makefile
+++ b/Makefile
@@ -1,20 +1,21 @@
-include config.mk
+.POSIX:
 
-.PHONY: all clean install test
+include config.mk
 
 all: scroll
+
 clean:
rm -f scroll ptty
 
 config.h:
cp config.def.h config.h
 
-scroll: scroll.c config.h
-   $(CC) $(CFLAGS) $(CPPFLAGS) scroll.c $(LDLIBS) -o $@
+scroll: config.h
 
 install: scroll
-   cp scroll ${BINDIR}
-   cp scroll.1 ${MAN1DIR}
+   mkdir -p $(BINDIR) $(MANDIR)/man1
+   cp -f scroll $(BINDIR)
+   cp -f scroll.1 $(MANDIR)/man1
 
 test: scroll ptty
# check usage
@@ -22,3 +23,8 @@ test: scroll ptty
# check exit passthrough of child
if ! ./ptty ./scroll true;  then exit 1; fi
if   ./ptty ./scroll false; then exit 1; fi
+
+.c:
+   $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -o $@ $< -lutil
+
+.PHONY: all clean install test
diff --git a/config.mk b/config.mk
index 20fa68c..247bb01 100644
--- a/config.mk
+++ b/config.mk
@@ -1,10 +1,9 @@
 # paths
 PREFIX = /usr/local
-BINDIR = ${PREFIX}/bin
-MANDIR = ${PREFIX}/share/man
-MAN1DIR= ${MANDIR}/man1
+BINDIR = $(PREFIX)/bin
+MANDIR = $(PREFIX)/share/man
 
-CC ?= cc
-CFLAGS = -std=c99 -pedantic -Wall -Wextra -g
-LDLIBS += -lutil
-CPPFLAGS += -D_DEFAULT_SOURCE
+CPPFLAGS = -D_DEFAULT_SOURCE
+# if your system is not POSIX, add -std=c99 to CFLAGS
+CFLAGS = -Os
+LDFLAGS = -s
-- 
2.26.0

[hackers] [scroll][PATCH 2/2] Add distclean make target

[Quentin Rameau]

---
 Makefile | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c3e1c50..4d415b7 100644
--- a/Makefile
+++ b/Makefile
@@ -4,9 +4,6 @@ include config.mk
 
 all: scroll
 
-clean:
-   rm -f scroll ptty
-
 config.h:
cp config.def.h config.h
 
@@ -24,7 +21,13 @@ test: scroll ptty
if ! ./ptty ./scroll true;  then exit 1; fi
if   ./ptty ./scroll false; then exit 1; fi
 
+clean:
+   rm -f scroll ptty
+
+distclean: clean
+   rm -f config.h
+
 .c:
$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -o $@ $< -lutil
 
-.PHONY: all clean install test
+.PHONY: all install test clean distclean
-- 
2.26.0

Re: [hackers] [st][PATCH] Rearrange x run with fast exit

[Quentin Rameau]

> I separate unrelated modifications into unrelated commits but this is
> still related, I think it also belongs under reorderings.

That was the polite way of saying “They're unrelated, separate them in
two commits”, apparently polite messages don't convey the meaning.

> On Wed, Mar 18, 2020 at 02:10:36PM +0100, Quentin Rameau wrote:
> >> Quickly swap out of loop to reduce indentation needed.
> >> Reuse deltatime variable to reuse calculation.
> >
> >Don't be afraid to separate unrelated modifications into separate
> >commits!
> >
> 

Re: [hackers] [st][PATCH] Rearrange x run with fast exit

[Quentin Rameau]

> Quickly swap out of loop to reduce indentation needed.
> Reuse deltatime variable to reuse calculation.

Don't be afraid to separate unrelated modifications into separate
commits!

Re: [hackers] [PATCH][sbase] paste: Allow null delim

[Quentin Rameau]

> This has nothing to do with getopt(). This is about how the shell parses
> and splits words into arguments. -d""

Erf of course, put that on the fever. ^^

Re: [hackers] [PATCH][sbase] paste: Allow null delim

[Quentin Rameau]

On Thu, 5 Mar 2020 01:42:37 -0800
Michael Forney  wrote:

> On 2020-03-05, Quentin Rameau  wrote:
> >> Looking at POSIX, I see that `-d '\0'` must be supported, `-d ""`
> >> is unspecified  
> >
> > I don't think so, -d "" is just a list with an empty string.
> > So -d '\0' is equivalent to -d '', '\0' is here to let the user
> > express an empty string in a list, which wouldn't be possible
> > otherwise (like how would one specify empty string in a list like
> > 'abcd').  
> 
> Here is a direct quote from POSIX:
> 
> The commands:
> 
> paste -d "\0" ...
> paste -d "" ...
> 
> are not necessarily equivalent; the latter is not specified by
> this volume of POSIX.1-2017 and may result in an error.

My bad, I didn't read that far.

But,

> >> and `-d""` is invalid, since paste(1) must follow the
> >> utility syntax guidelines (guideline 7).  
> >
> > Not sure what you mean there, -d"" is the concatenation of -d and
> > '', which is standard.
> > Did you quote the correct guideline? “Guideline 7: Option-arguments
> > should not be optional.” here there's an option-argument, that's an
> > empty string.  
> 
> I guess it's a combination of 6 and 7. Option arguments must be
> separate parameters and non-optional (unless specified otherwise).
> There is an exception made that allows conforming implementations to
> accept an option adjacent with its option argument in the same
> argument, but I think this only makes sense if the option-argument is
> non-empty.
> 
> POSIX says
> 
>   The construct '\0' is used to mean "no separator" because historical
> versions of paste did not follow the syntax guidelines, and the
> command:
> 
>   paste -d"" ...
> 
>   could not be handled properly by getopt().
> 
> I think this implies that `paste -d""` is no longer valid, due to the
> requirements of the syntax guidelines.

Rather it's due to a getopt() limitation with the (not so) special case
of an empty string option-argument, but I'm not sure why.

The utility guidelines permits concatenating options and
option-arguments, while it's true that the prefered syntax is to
separate them.

12.1, 2.a:
“If the SYNOPSIS of a standard utility shows an option with a mandatory
option-argument (as with [ -c option_argument] in the example), a
conforming application shall use separate arguments for that option and
its option-argument. However, a conforming implementation shall also
permit applications to specify the option and option-argument in the
same argument string without intervening  characters.”

Without commenting on the patch itself, I think it's fine to allow '\0'
and '', especially as we don't use getopt(), partly due to it being
broken in most implementations (and having such limitations?), and the
standards says it's unspecified, not undefinied.

Re: [hackers] [PATCH][sbase] paste: Allow null delim

[Quentin Rameau]

Hello all,

> Looking at POSIX, I see that `-d '\0'` must be supported, `-d ""` is
> unspecified

I don't think so, -d "" is just a list with an empty string.
So -d '\0' is equivalent to -d '', '\0' is here to let the user express
an empty string in a list, which wouldn't be possible otherwise (like
how would one specify empty string in a list like 'abcd').

> and `-d""` is invalid, since paste(1) must follow the
> utility syntax guidelines (guideline 7).

Not sure what you mean there, -d"" is the concatenation of -d and '',
which is standard.
Did you quote the correct guideline? “Guideline 7: Option-arguments
should not be optional.” here there's an option-argument, that's an
empty string.

[hackers] [PATCH-v2] od: set -tx default byte number to integer size

[Quentin Rameau]

Scratch the above patch, I forgot to credit prez for the report.
Please use the attached patch!

Thanks Michael
>From f3eef8e9c4820b7764a229fa40d714f52fc676ee Mon Sep 17 00:00:00 2001
From: Quentin Rameau 
Date: Wed, 4 Mar 2020 17:02:15 +0100
Subject: [PATCH] od: set -tx default byte number to integer size

Thanks to prez  for the report and
base patch!
---
 od.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/od.c b/od.c
index 0b1c5c6..9ff8ff2 100644
--- a/od.c
+++ b/od.c
@@ -212,7 +212,7 @@ main(int argc, char *argv[])
 {
int fd;
struct type *t;
-   int ret = 0, len;
+   int ret = 0, len, defbytes;
char *s;
 
big_endian = (*(uint16_t *)"\0\xff" == 0xff);
@@ -260,6 +260,7 @@ main(int argc, char *argv[])
case 'o':
case 'u':
case 'x':
+   defbytes = 0;
/* todo: allow multiple digits */
if (*(s+1) > '0' && *(s+1) <= '9') {
len = *(s+1) - '0';
@@ -271,17 +272,17 @@ main(int argc, char *argv[])
case 'S':
len = sizeof(short);
break;
+   default:
+   defbytes = 1;
case 'I':
len = sizeof(int);
break;
case 'L':
len = sizeof(long);
break;
-   default:
-   len = sizeof(int);
}
}
-   addtype(*s++, len);
+   addtype(defbytes ? *s : *s++, len);
break;
default:
usage();
-- 
2.25.1

[hackers] [PATCH] od: set -tx default byte number to integer size

[Quentin Rameau]

---
 od.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/od.c b/od.c
index 0b1c5c6..9ff8ff2 100644
--- a/od.c
+++ b/od.c
@@ -212,7 +212,7 @@ main(int argc, char *argv[])
 {
int fd;
struct type *t;
-   int ret = 0, len;
+   int ret = 0, len, defbytes;
char *s;
 
big_endian = (*(uint16_t *)"\0\xff" == 0xff);
@@ -260,6 +260,7 @@ main(int argc, char *argv[])
case 'o':
case 'u':
case 'x':
+   defbytes = 0;
/* todo: allow multiple digits */
if (*(s+1) > '0' && *(s+1) <= '9') {
len = *(s+1) - '0';
@@ -271,17 +272,17 @@ main(int argc, char *argv[])
case 'S':
len = sizeof(short);
break;
+   default:
+   defbytes = 1;
case 'I':
len = sizeof(int);
break;
case 'L':
len = sizeof(long);
break;
-   default:
-   len = sizeof(int);
}
}
-   addtype(*s++, len);
+   addtype(defbytes ? *s : *s++, len);
break;
default:
usage();
-- 
2.25.1

Re: [hackers] [sbase][PATCH] od: Fix argument parsing for -t flag

[Quentin Rameau]

Hi prez,

> when the -t flag is used without a number of bytes, od exits with
> usage(). 'od -tx foo' should behave like 'od -tx4 foo'.

Indeed,

> ---
>  od.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/od.c b/od.c
> index 0b1c5c6..1780710 100644
> --- a/od.c
> +++ b/od.c
> @@ -250,6 +250,7 @@ main(int argc, char *argv[])
>   break;
>   case 't':
>   s = EARGF(usage());
> + int done = 0;
>   for (; *s; s++) {
>   switch (*s) {
>   case 'a':
> @@ -277,11 +278,15 @@ main(int argc, char *argv[])
>   case 'L':
>   len = sizeof(long);
>   break;
> - default:
> + case '\0':
>   len = sizeof(int);
> + done = 1;
> + break;
> + default:
> + usage();
>   }
>   }
> - addtype(*s++, len);
> + addtype(*(done ? s : s++), len);
>   break;
>   default:
>   usage();

This is slightly wrong though, we don't want to error on default
because it's allowed to concatenate multiple types.

I think something like that is better, what do you think?

diff --git a/od.c b/od.c
index 0b1c5c6..9ff8ff2 100644
--- a/od.c
+++ b/od.c
@@ -212,7 +212,7 @@ main(int argc, char *argv[])
 {
int fd;
struct type *t;
-   int ret = 0, len;
+   int ret = 0, len, defbytes;
char *s;
 
big_endian = (*(uint16_t *)"\0\xff" == 0xff);
@@ -260,6 +260,7 @@ main(int argc, char *argv[])
case 'o':
case 'u':
case 'x':
+   defbytes = 0;
/* todo: allow multiple digits */
if (*(s+1) > '0' && *(s+1) <= '9') {
len = *(s+1) - '0';
@@ -271,17 +272,17 @@ main(int argc, char *argv[])
case 'S':
len = sizeof(short);
break;
+   default:
+   defbytes = 1;
case 'I':
len = sizeof(int);
break;
case 'L':
len = sizeof(long);
break;
-   default:
-   len = sizeof(int);
}
}
-   addtype(*s++, len);
+   addtype(defbytes ? *s : *s++, len);
break;
default:
usage();

Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags

[Quentin Rameau]

Hi Laslo,

> > These tools are not standardized, but these flags are supported in all
> > implementations I'm aware of (coreutils, busybox), and are
> > occasionally used in scripts.
> > 
> > These flags are only meaningful on operating systems which
> > differentiate between text and binary files, so just ignore them.  
> 
> I would print something on stderr. POSIX is ignored often enough and a
> ton of scripts are using the cancerous GNU extensions and other
> extensions. If we just "ignore" them, there is no learning effect or
> push for change for script writers, so maybe we could add a warning
> while we ignore them, so when you run a script that makes use of these
> mostly useless flags (which we could also tell them), then this might a
> push in a good direction. What do you think?

I agree in that silently ignoring commands from the user is bad, as it
breaks expectations.

Though as noted in this case, those are not standardized (maybe that
wasn't a great idea to add them in sbase instead of ubase even if they
can be implemented in a portable manner), so anything can happen there.

I'm not sure we should start adding those kind of half-compability
parsing with coreutils, where do we stop?

Also for what it's worth:

$ busybox md5sum -b md5sum.c
md5sum: unrecognized option: b
BusyBox v1.31.1 (2019-11-29 10:55:12 UTC) multi-call binary.

Usage: md5sum [FILE]...

$ busybox md5sum -t md5sum.c
md5sum: unrecognized option: t
BusyBox v1.31.1 (2019-11-29 10:55:12 UTC) multi-call binary.

Usage: md5sum [FILE]...

Re: [hackers] [sbase] libutil/recurse: Use while-loop instead of for-loop with only condition || Michael Forney

[Quentin Rameau]

> Author: Michael Forney 

Hi Michael,

> libutil/recurse: Use while-loop instead of for-loop with only condition

The patch itself shows that, could you explain what it fixes or improve
in your commit messages?

Thanks.

> diff --git a/libutil/recurse.c b/libutil/recurse.c
> index d2dc3ed..487faac 100644
> --- a/libutil/recurse.c
> +++ b/libutil/recurse.c
> @@ -96,7 +96,7 @@ recurse(const char *path, void *data, struct recursor *r)
>   if (!(r->flags & DIRFIRST))
>   r->fn(path, , data, r);
>  
> - for (; r->hist; ) {
> + while (r->hist) {
>   h = r->hist;
>   r->hist = r->hist->prev;
>   free(h);
> 

Re: [hackers] [dwm][PATCH] cleanup, saves 7 LOC

[Quentin Rameau]

> As of [0], the Xorg man page for XFree reads:
> 
> "If data is NULL, no operation is performed."
> 
> It was changed to reflect the behavior that Xorg has always had.
> XFree() has been #defined to free() since their initial code import in
> 2003.  However, it looks like other documentation was not updated, and
> it's understandable if you want to keep the NULL checks.
> 
> [0]: https://gitlab.freedesktop.org/xorg/lib/libx11/commit/78851f6

Agreed.

> > I think the current sendevent() code is more readable.

Agreed too.

Please provide both changes as separate commits, one for XFree()
unconditional call, and another one for refactoring sendevent().

Thanks!

Re: [hackers] [sbase] yes: Simplify, only support one argument || Michael Forney

[Quentin Rameau]

> yes: Simplify, only support one argument
> 
> The previous code was too difficult to decipher for such a simple tool.

Agreed!

Re: [hackers] [sbase] Add .gitignore || Michael Forney

[Quentin Rameau]

> > yuck  
> 
> Remarks like this aren't constructive, please keep them to yourself.

Come on, don't be butthurted.

> I already justified why adding .gitignore and rule to regenerate it is
> useful. I'm sorry that you don't approve, but not having a .gitignore
> was a significant annoyance to me and was making it difficult to do my
> job as maintainer.

No, you argued your point, that's different, and it did not answer my
point about it being just a bloat for some lazy convenience; I'm
talking about the redundancy of having both the actual file tracked
in the SCM and a script to generate it instead of manually editing it.

> When I asked you about your workflow to try to understand your
> opposition to .gitignore, you ignored my question and *at the same
> time* accused me of ignoring your question, which I had answered
> multiple times at that point.

This is not about *me* (or my workflow), nor is it about *you* (or your
workflow), but that's what you did nevertheless.

> I'm tired of arguing with you, so please, just find a way to deal with it.

Nice collaborating with you.

Re: [hackers] [sbase] Add .gitignore || Michael Forney

[Quentin Rameau]

> Add .gitignore
> 
> Also, add rule to regenerate in case executable list changes.

yuck

Re: [hackers] [sbase][PATCH] Minor optimizations for 'yes'

[Quentin Rameau]

> Dear Quentin,

Hi Laslo,

> > I agree, yes is neither a benchmark tool nor a data generator, it just
> > outputs 'y' for piping into other commands.
> > Keep it simple.
> 
> I agree with you in general, but are we really "optimizing" yes(1) here
> for the sake of performance? This looks to me like a case of premature
> optimization.

To be honest yes, this reads as bikeshedding, but on the other hand, a
simple tool as yes is quite complicated, at least for an sbase
implementation.

> We don't know if a script relies on the behaviour of yes(1) printing
> all passed strings repeatedly. So, even though the tool is not
> standardized by Posix the common "consensus" is that
> 
>   - with no arguments, it shall print y and a newline repeatedly.
>   - with arguments, it shall print them, comma separated, followed by
> newline, repeatedly.

Where did you get this was the common consensus?
This is the GNU behaviour, ie not a consensus.

The yes(1) rationale is to output a 'y' character for piping into
interactive tool's stdin and make them batchable.
The tool lets you output a custom string which would be passed to its
argument, so when a user wants yes to output anything else, it's a
simple matter of passing it on the command line.

The consensus here is that yes takes a parameter, at least one.
So if one wants to output a string, that's simple:

yes 'my very long string with arguments'
or
yes "$foo $bar $baz"

Done.

> The point about readability is a good one. I will even take the blame
> for writing it as it is now. However, you could greatly improve
> readability without sacrificing features/performance. The line
> 
>   argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0;
> 
> is used in many places in sbase and is more or less an idiom. The
> entire tool itself could then be written as follows. We can even
> remove the checks consequently within the loop, which could be regarded
> as a "performance" improvement.
> 
>   int i;
> 
>   argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0;
>   
>   if (argc == 0) {
>   for (;;) {
>   fputs("y\n", stdout);
>   }
>   } else {
>   for (;;) {
>   for (i = 0; i < argc; i++) {
>   fputs(argv[i], stdout);
>   fputc(' ', stdout);
>   }
>   fputc('\n', stdout);
>   }
>   }

And then this becomes quite simple, if yes gets argc > 1, printf
argv[1], otherwise print 'y'.

> What do you think? Patch is attached.

I think this answers the question ;)

Re: [hackers] [sbase][PATCH] Minor optimizations for 'yes'

[Quentin Rameau]

Hello,

> So, we could make this as simple as
> 
>   s = argc > 1 ? argv[1] : "y";
>   for (;;)
>   puts(s);
> 
> >> I think I'd prefer
> >>
> >>for (;;)
> >>puts("y");
> >>
> >> here.  
> > Originally, I had a puts(3) call there, but this decreased the performance
> > from the unpatched sbase, so I changed it to some fputc calls. Calling
> > write(2) didn't help either. I researched why and it's due to some
> > buffering,
> > but adding buffering would overcomplicate the code.  
> 
> I think the major slowdown here is from locking and unlocking the file
> for every loop iteration. Using putchar_unlocked will be much faster
> than a plain putchar. But anyway, I think we should stick to puts(3)
> for simplicity.

I agree, yes is neither a benchmark tool nor a data generator, it just
outputs 'y' for piping into other commands.
Keep it simple.

Re: [hackers] [sbase] [PATCH] Add .gitignore

[Quentin Rameau]

> On 2019-06-17, Quentin Rameau  wrote:
> >> How do you deal with ~250 lines of "Untracked files:" in the `git
> >> status` output?
> >>
> >> If you want to see them, you can always run `git status --ignored`.
> >
> > If you want it, you can always run `make .gitignore` *once*.
> 
> Once for every clone of sbase.

Yes, as opposed to once for every git status command issued.

> >> >> The place for user-specific and repository-specific ignored files is
> >> >> .git/info/exclude. But in general, we don't know the location of the
> >> >> .git directory, so we'd probably have to use some git command to
> >> >> figure out exactly where to put it.
> >> >
> >> > There's no “we”, that's the user's responsability to figure out where
> >> > to put it.
> >>
> >> There is a "we" if a Makefile rule were to be added, since it needs to
> >> create the file at the appropriate location.
> >
> > The appropriate location is .gitignore.
> 
> No, see gitignore(5):
> 
> * Patterns which should be version-controlled and distributed to other
>   repositories via clone (i.e., files that all developers will want to
>   ignore) should go into a .gitignore file.
> 
> * Patterns which are specific to a particular repository but which do
>   not need to be shared with other related repositories (e.g., auxiliary
>   files that live inside the repository but are specific to one user’s
>   workflow) should go into the $GIT_DIR/info/exclude file.
> 
> * Patterns which a user wants Git to ignore in all situations (e.g.,
>   backup or temporary files generated by the user’s editor of choice)
>   generally go into a file specified by core.excludesFile in the user’s
>   ~/.gitconfig. Its default value is $XDG_CONFIG_HOME/git/ignore. If
>   $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/ignore
>   is used instead.
> 
> >> > What's the rationale for having it duplicated both in the SCM and in
> >> > the
> >> > Makefile then?
> >>
> >> So that if a utility is added or removed, .gitignore can easily be kept in
> >> sync.
> >
> > So what's the point of having .gitignore tracked by the SCM?
> 
> So that it gets applied by default.

It seems you're deliberately not answering the question, or are
suggesting bloating the make/SCM system with a justification for
lazyness.

You asked if there was any objection to this, you got some, now do what
you want.

But pushing it both into the Makefile and the SCM is not justified at
all, chose either one.

Re: [hackers] [sbase] [PATCH] Add .gitignore

[Quentin Rameau]

On Mon, 17 Jun 2019 13:40:47 -0700
Michael Forney  wrote:

> On 2019-06-17, Quentin Rameau  wrote:
> >> What's the downside to having it checked into the repository? Are
> >> there any reasons why a user wouldn't want this feature? Personally, I
> >> want it in every clone of sbase I make, and I imagine most other
> >> people want this as well. It is annoying to have `git status` scroll
> >> my changes off the screen due to all the build artifacts.  
> >
> > Well, personnaly I don't want it, I prefer to have a clear view of the
> > clone status, hence the “user-side feature”, as in user-specific
> > preference.  
> 
> How do you deal with ~250 lines of "Untracked files:" in the `git
> status` output?
> 
> If you want to see them, you can always run `git status --ignored`.

If you want it, you can always run `make .gitignore` *once*.

> >> The place for user-specific and repository-specific ignored files is
> >> .git/info/exclude. But in general, we don't know the location of the
> >> .git directory, so we'd probably have to use some git command to
> >> figure out exactly where to put it.  
> >
> > There's no “we”, that's the user's responsability to figure out where
> > to put it.  
> 
> There is a "we" if a Makefile rule were to be added, since it needs to
> create the file at the appropriate location.

The appropriate location is .gitignore.
> 
> > What's the rationale for having it duplicated both in the SCM and in the
> > Makefile then?  
> 
> So that if a utility is added or removed, .gitignore can easily be kept in 
> sync.

So what's the point of having .gitignore tracked by the SCM?

Re: [hackers] [sbase] [PATCH] Add .gitignore

[Quentin Rameau]

> > I'm not really keen on pushing this file to the source history, I think
> > this should be kept separate, this is a user-side feature.
> 
> What's the downside to having it checked into the repository? Are
> there any reasons why a user wouldn't want this feature? Personally, I
> want it in every clone of sbase I make, and I imagine most other
> people want this as well. It is annoying to have `git status` scroll
> my changes off the screen due to all the build artifacts.

Well, personnaly I don't want it, I prefer to have a clear view of the
clone status, hence the “user-side feature”, as in user-specific
preference.

> > But having a rule un the Makefile, as you did, is a good compromise
> > there, if the user wants it, he can have it with little additionnal
> > effort.
> >
> > So I'd be for pushing the Makefile rule, not committing the actual file
> > into the development history
> 
> The place for user-specific and repository-specific ignored files is
> .git/info/exclude. But in general, we don't know the location of the
> .git directory, so we'd probably have to use some git command to
> figure out exactly where to put it.

There's no “we”, that's the user's responsability to figure out where
to put it.

What's the rationale for having it duplicated both in the SCM and in the
Makefile then?

Re: [hackers] [sbase] [PATCH] Add .gitignore

[Quentin Rameau]

Hello Michael,

> Also, add rule to regenerate in case executable list changes.
> ---
> Any objection to this?

I'm not really keen on pushing this file to the source history, I think
this should be kept separate, this is a user-side feature.

But having a rule un the Makefile, as you did, is a good compromise
there, if the user wants it, he can have it with little additionnal
effort.

So I'd be for pushing the Makefile rule, not committing the actual file
into the development history

Re: [hackers] [sbase] [PATCH] libutil: Rename functions in reserved namespace to prevent potential conflict

[Quentin Rameau]

> strsep is reserved.
> 
> C11 standard section 7.30 Future library directions:
> 
> The following names are grouped under individual headers for
> convenience. All external names described below are reserved no matter
> what headers are included by the program.
> 
> 7.30.10 String handling 
> 
> Function names that begin with str, mem, or wcs and a lowercase letter
> may be added to the declarations in the  header.

Indeed, I stand corrected (already present in C99)!

Re: [hackers] [sbase] [PATCH] libutil: Rename functions in reserved namespace to prevent potential conflict

[Quentin Rameau]

> libc may define functions with the same names, but differing in prototype.
> ---
> Anyone have any comments on this patch before I push it?
> 
> The motivating conflict is with strsep, which glibc declares as
> 
>   char *strsep(char **restrict, const char *restrict);
> 
> which is not compatible with the libutil declaration.
> 
>   char *strsep(char **, const char *);

x* is not a reserved namespace, nor is strsep.

libc is always linked last, so this this won't cause an issue.

Re: [hackers] [dwm][PATCH] hide_vacant_tags patch

[Quentin Rameau]

On Fri, 29 Mar 2019 15:32:19 +0100
Ryan Kes  wrote:

> ---
>  dwm.c | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)

Hello Ryan,

You can update those patches yourself, I suggest having a look at
http://suckless.org/wiki/

Re: [hackers] [st][patch] be silent about explicitly unhandled mouse modes

[Quentin Rameau]

Hello Abdullah,

> it fixes this.
> 
> see attachment. 
> 
> On 13/03, Quentin Rameau wrote:
> > Hello Lauri,
> > 
> > > diff below fixes it.
> > 
> > What does it fix?
> > 

Please do not send to this mailing list while being drunk.

Re: [hackers] [st][patch] be silent about explicitly unhandled mouse modes

[Quentin Rameau]

> the erroneous output on stderr. the comment reads like these escapes are
> intentionally left unhandled, but in that case it doesn't make sense to
> output an error about them.

Ok, I thought there was actually an issue preventing tmux to work
correctly.

Re: [hackers] [st][patch] be silent about explicitly unhandled mouse modes

[Quentin Rameau]

Hello Lauri,

> diff below fixes it.

What does it fix?

Re: [hackers] [dev][slstatus] FreeBSD port?

[Quentin Rameau]

> Hi,

Hi Michael,

> As part of my setup of FreeBSD I ported slstatus. Is the mainline a place
> to commit such patches given the platform dependence of a tool like
> slstatus? I guess porting to new platforms can increase the size of the
> code base significantly if one sticks to the current project structure. Or
> is a port like this better suited simply as a fork?

The place would be indeed the suckless repo as it already had the goal
to support multiple OSes.

Maybe then, we could revert to having a source/object file per target!

Re: [hackers] Re: [st][patch] Refresh keyboard mapping

[Quentin Rameau]

> > Interesting but I can't reproduce any issue with that, could you
> > provide a way to raise a problem this way?
> 
> Expect it within 24 hours.

Ok, I started the (final) countdown.