From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] Fix for bug 12539 Date: Fri, 11 Dec 2020 09:45:53 +0100 Message-ID: <465AFCB3-F810-4BF1-9B7B-E6A0ECC16325@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2577601435881505179==" List-Id: --===============2577601435881505179== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Adolf, You changed some already messy code, so there is no point in changing variabl= e names. It would probably make the diff even more complicated and won=E2=80= =99t make a difference to the user anyways. With new code, I would use explicit variable names. Languages like Python for= ce you to do that (for x in y) and there is no way around it. I personally co= nsider that better language design. But it is great to have a conversation about this with you guys :) -Michael > On 10 Dec 2020, at 17:45, Adolf Belka wrote: >=20 > Hi, >=20 >=20 > On 10/12/2020 16:46, Michael Tremer wrote: >> Hi, >>> On 10 Dec 2020, at 16:26, Bernhard Bitsch wrot= e: >>>=20 >>>=20 >>>=20 >>>> Gesendet: Donnerstag, 10. Dezember 2020 um 15:48 Uhr >>>> Von: "Michael Tremer" >>>> An: "Bernhard Bitsch" >>>> Cc: "IPFire: Development-List" >>>> Betreff: Re: [PATCH] Fix for bug 12539 >>>>=20 >>>> Hello, >>>>=20 >>>>> On 10 Dec 2020, at 15:35, Bernhard Bitsch wr= ote: >>>>>=20 >>>>> Hi, >>>>>=20 >>>>>> Gesendet: Donnerstag, 10. Dezember 2020 um 14:29 Uhr >>>>>> Von: "Michael Tremer" >>>>>> An: "Bernhard Bitsch" >>>>>> Cc: "Adolf Belka" , development(a)lists.ipfire= .org >>>>>> Betreff: Re: [PATCH] Fix for bug 12539 >>>>>>=20 >>>>>> Hi, >>>>>>=20 >>>>>>> On 10 Dec 2020, at 14:13, Bernhard Bitsch = wrote: >>>>>>>=20 >>>>>>> Hello, >>>>>>>=20 >>>>>>>> Gesendet: Donnerstag, 10. Dezember 2020 um 13:48 Uhr >>>>>>>> Von: "Michael Tremer" >>>>>>>> An: "Adolf Belka" >>>>>>>> Cc: development(a)lists.ipfire.org >>>>>>>> Betreff: Re: [PATCH] Fix for bug 12539 >>>>>>>>=20 >>>>>>>> Hello, >>>>>>>>=20 >>>>>>>> Great work. This looks like how it should be. >>>>>>>>=20 >>>>>>>>> On 7 Dec 2020, at 15:01, Adolf Belka wrote: >>>>>>>>>=20 >>>>>>>>> The installer recognises cups and cups-filters both as cups and puts >>>>>>>>> two instances of cups in the add-on services table. >>>>>>>>> Based on input from Michael Tremer this patch replaces the command >>>>>>>>> returning the second element between hyphens with one that takes >>>>>>>>> what comes after "meta-" using Perl code rather than a shell comman= d. >>>>>>>>> The second find command was changed as per Michael's suggestion. >>>>>>>>>=20 >>>>>>>>> Tested in my ipfire test bed system and only results in one cups >>>>>>>>> entry. >>>>>>>>> Signed-off-by: Adolf Belka >>>>>>>>> --- >>>>>>>>> html/cgi-bin/services.cgi | 9 +++++---- >>>>>>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>>>>>>=20 >>>>>>>>> diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi >>>>>>>>> index 26ab4f314..36954ba70 100644 >>>>>>>>> --- a/html/cgi-bin/services.cgi >>>>>>>>> +++ b/html/cgi-bin/services.cgi >>>>>>>>> @@ -161,19 +161,20 @@ END >>>>>>>>> my $lines=3D0; # Used to count the outputlines to make different b= gcolor >>>>>>>>>=20 >>>>>>>>> # Generate list of installed addon pak's >>>>>>>>> - my @pak =3D `find /opt/pakfire/db/installed/meta-* 2>/dev/null | = cut -d"-" -f2`; >>>>>>>>> + opendir (DIR, "/opt/pakfire/db/installed") || die "Cannot opendir= /opt/pakfire/db/installed/: $!"; >>>>>>>>> + my @pak =3D sort readdir DIR; >>>>>>>>> foreach (@pak){ >>>>>>>>> chomp($_); >>>>>>>>> + next unless (m/^meta-/); >>>>>>>>> + s/^meta-//; >>>>>>>>=20 >>>>>>>> Although this is the least intuitive thing to do. I have no idea who= designed Perl. I hope they are proud of all the chaos they have created :) >>>>>>>>=20 >>>>>>>> -Michael >>>>>>>=20 >>>>>>> Can't see where the solution is 'least intuitive'. As far as I can se= e from the patch, now the directory of installed addons is read with Perl fun= ctions ( not in a extra forked process ). The definition of filenames is used= consequently ( 'meta-', possible services are stored in /etc/ini= t.d/ ). >>>>>>=20 >>>>>> To be clear, I am complaining about the Perl language design instead o= f the code. In my view something like >>>>>>=20 >>>>>> name =3D name.replace(=E2=80=9Cmeta-=E2=80=9C, =E2=80=9C=E2=80=9D) >>>>>>=20 >>>>>> is a thousand times easier to read. >>>>>>=20 >>>>>> Best, >>>>>> -Michael >>>>>>=20 >>>>>=20 >>>>> Okay, that makes it clearer. >>>>> Perl has its roots in pattern matching. So the base functions of matchi= ng and replacing are defined with minimal overhead in writing. >>>>> Maybe the designer(s) of Perl had in mind the description of COBOL as "= speaking english with your computer". This was in the beginnings of computing= , which did not experience all of us. ;) >>>>> With this single example you are right. Your formulation is much more r= eadable ( besides the fact that name is a string object ). But if you have ma= ny such snippets in your code, the Perl way makes a shorter and thus more und= erstandable program. >>>>=20 >>>> I disagree. Why would the usage of something that is hard to comprehend = be better when it is being used more often? Getting used to the wrong pattern= doesn=E2=80=99t make it right. >>>>=20 >>>> My main complaint about this is, that it is not clear what is being mani= pulated. It is better to use the variable name and then there is no confusion= about it. The same is true for loads of other Perl-isms like shift, chomp, c= hop, etc. >>>>=20 >>>> Shorter code does not necessarily mean that it executes faster. Probably= the opposite is true in this case, because you would have to compile a regul= ar expression first and the Python version would simply perform a string sear= ch. >>>>=20 >>>> The point is, that code should be obvious. It avoids bugs and it makes i= t easier to understand for reviewers what is supposed to go on. >>>>=20 >>>=20 >>> The main difference between the Perl and the Python approach is the basic= language class. Python works on objects and with their methods ( functions )= , Perl works on plain variables with operators. Types are determined by the o= perator and context. Each operator has also a result which isn't destroyed by= a assigment or statement end. This result is hold in a special variable whic= h can be manipulated directly. >>> If I review code, I should know this basics. Thus it doesn't make a real = difference in comprehension ( to me ) if I read a object-oriented Python code= or a Perl code. Nevertheless is the Perl code shorter. In our special exampl= e Perl avoids the generation (in written code ) of an intermeditiate variable= by use of the implicit $_. >>> But this is a case of habit. Some like to talk in English only, some like= to talk in their native language to be more exact. ;) >> Yes, I understand that. But what do you do when you have more than one var= iable? Most of my code does. >=20 > My Perl learning here included the existence of the implicit $_. I let it i= n place because the section covered by foreach (@pak) was using it. > However if the preference is to be explicit with variables, I have no probl= em in doing that in any future bug fixes etc that I do. >=20 > Regards, > Adolf. >=20 >>> Best, >>> Bernhard >>>> Best, >>>> -Michael >>>>=20 >>>>>=20 >>>>> Best, >>>>> Bernhard >>>>>=20 >>>>>>>=20 >>>>>>> - Bernhard >>>>>>>=20 >>>>>>>=20 >>>>>>>>=20 >>>>>>>>>=20 >>>>>>>>> # Check which of the paks are services >>>>>>>>> - my @svc =3D `find /etc/init.d/$_ 2>/dev/null | cut -d"/" -f4`; >>>>>>>>> - foreach (@svc){ >>>>>>>>> + if (-e "/etc/init.d/$_") { >>>>>>>>> # blacklist some packages >>>>>>>>> # >>>>>>>>> # alsa has trouble with the volume saving and was not really sto= pped >>>>>>>>> # mdadm should not stopped with webif because this could crash t= he system >>>>>>>>> # >>>>>>>>> - chomp($_); >>>>>>>>> if ( $_ eq 'squid' ) { >>>>>>>>> next; >>>>>>>>> } >>>>>>>>> -- >>>>>>>>> 2.29.2 --===============2577601435881505179==--