grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] misc: Allow selective disabling of debug facility names


From: Daniel Kiper
Subject: Re: [PATCH v2] misc: Allow selective disabling of debug facility names
Date: Thu, 9 Dec 2021 19:41:43 +0100
User-agent: NeoMutt/20170113 (1.7.2)

On Tue, Dec 07, 2021 at 06:27:26PM -0600, Glenn Washburn wrote:
> On Tue, 7 Dec 2021 23:07:32 +0100
> Michael Schierl <schierlm@gmx.de> wrote:
>
> > Hello Glenn,
> >
> >
> > Am 07.12.2021 um 22:59 schrieb Glenn Washburn:
> > > Yes, but I didn't want to assume that "all" is the first item in the
> > > list.
> >
> > More important, "all" does not have to be present at all.
> >
> > DEBUG=fat,btrfs,-fat
> >
> > should be perfectly legal (and it is).
>
> Agreed.
>
> > > I decided that it doesn't make as much sense to have the position of
> > > "all" matter, and effectively be a macro for all conditionals, because
> > > at that point why append ",all,...", just set debug to "all,..."
> > > erasing the previous contents.
> >
> > That is a fair point. Same applies to "-all" (just erase everything in
> > the string before it).
> >
> > Instead of
> >
> > DEBUG=btrfs,all,fat,-all,ext2
> >
> > probably the user just wanted to enter
> >
> > DEBUG=ext2
>
> Yep.
>
> > > I'm open to suggestions on what the preferred way to handle this might
> > > be. We could say that "all" must appear at the beginning, but that
> > > seems unnecessary and overly restrictive. What's less clear to me is
> > > what situation I might want to put have "all" not at the beginning of
> > > the string. I'm thinking there might be one, which is why I'm leaving
> > > that as an option, but maybe I'm wrong.
> >
> > I cannot think of any sensible one that would not confuse the user, like
> > your example with
> >
> > DEBUG=-btrfs,all,-fat
> >
> > But I don't think we have to put too much effort in it, especially since
> > the code should also work in case "all" is not present at all.
>
> The other thing is that the original code allows "all" to be any item
> in the string. So not allowing that would break backward-compatibility
> (maybe not a big deal though). And I also agree that it can be
> confusing. Maybe Daniel has an opinion.

It would be nice to have backward compatibility but I do not think it is
a must here. If we have to drop backward compatibility to provide
better functionality I am OK with it. For me the implementation of this
feature has to be efficient, i.e. do not delay execution strongly,
the configuration has to be easy to understand and well documented.

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]