Hello Quentin,

On Sun, Nov 29, 2020 at 07:38:08PM +0100, Quentin Rameau wrote:
> 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?
> 

It is not really the current behaviour as the current behaviour is
static. In your example, if you enter the letter b, "foo" will be
filtered out so there will be only 2 items left but the dmenu window
will still have 3 lines. My first patch addresses that: when the number
of items is less than the initial number of lines, the actual number of
lines is reduced.

> 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.
> 

Sorry about that, I took a quick look at the coding style but I missed
that point. I'll update that in a v2 if you think that this first patch
should be added to mainline.


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

I took a look at the wiki and I think that the xyw patch is better than
mine and solves the same issue. So my second patch can be discarded.

> 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.
> 

I get your point on using only one letter. However, the relevant letters
('f, 'w', 'p') are already used and since dmenu has some options with 2
letters ('-fn', '-nb',...) I thought it was more consistent with other
options.
But as I said, this second patch may be discarded.

> Thanks again, keep on working and improving!
> 

You're welcome!


Also, there is one thing that I don't really understand: how to know if
a patch should be included in mainline or should be added on the wiki?
According to the hacking page of the website, the patches that fit
personal taste are on the wiki and others are pushed on mainline. But
how some patches that adds option like the xyw patch are more personal
taste than font and colors options that are part of mainline ? This
patch adds options but does not change the default behaviour.


Thank you for the review,
Jérémy

Reply via email to