From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:53386) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hX9OF-0005m5-64 for guix-patches@gnu.org; Sat, 01 Jun 2019 15:10:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hX9OD-0002u3-Vv for guix-patches@gnu.org; Sat, 01 Jun 2019 15:10:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:53373) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hX9OD-0002tu-TO for guix-patches@gnu.org; Sat, 01 Jun 2019 15:10:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hX9OD-0002kv-MF for guix-patches@gnu.org; Sat, 01 Jun 2019 15:10:01 -0400 Subject: [bug#35790] [PATCH] scripts: lint: Handle warnings with a record type. Resent-Message-ID: References: <20190518093206.22069-1-mail@cbaines.net> <878suz27ke.fsf@gnu.org> From: Christopher Baines In-reply-to: <878suz27ke.fsf@gnu.org> Date: Sat, 01 Jun 2019 20:09:02 +0100 Message-ID: <87ef4dxgvl.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 35790@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Hello! > > Christopher Baines skribis: > >> Rather than emiting warnings directly to a port, have the checkers retur= n the >> warning or warnings. >> >> This makes it easier to use the warnings in different ways, for example, >> loading the data in to a database, as you can work with the >> records directly, rather than having to parse the output to determine the >> package and location. > > Yay! > >> + > > As a rule of thumb, it=E2=80=99s best to not export the record type descr= iptor > (RTD) because then anything could happen. In this case, I think the > tests would be just as readable if we used =E2=80=98lint-warning-message= =E2=80=99 & > co. instead of matching on the record. > > WDYT? Interesting. I've now adjusted the tests accordingly and sent an updated patch. I've stuck with using match, as this gives much better error messages than using car, or lint-warning-message without checking the thing your working with is actually a list with a single warning. I've wrapped this up as a single-lint-warning-message that many of the tests use. >> +(define* (make-warning package message >> + #:key field location) >> + (make-lint-warning >> + package >> + message > > In practice MESSAGE is already translated. I think it would be more > flexible if it were not; =E2=80=98lint-warning-message=E2=80=99 would alw= ays return the > English message, and it=E2=80=99d be up to the user to call =E2=80=98gett= ext=E2=80=99 on it, > like we do for package descriptions. > > To achieve this, you=E2=80=99d need a little trick so that =E2=80=98xgett= ext=E2=80=99 can still > extract the messages, like: > > > (define-syntax-rule make-warning > (syntax-rule (G_) > ((_ package (G_ message) rest ...) > (%make-warning package message rest ...)))) > > where =E2=80=98%make-warning=E2=80=99 is the procedure you define above. > > Then you need an explicit call to =E2=80=98G_=E2=80=99 at the point where= messages are > displayed. > > Does that make sense? Yes, but I'm unsure it'll work for all the messages. Some of them it translates a format string first, then uses that format string, and that becomes the message, e.g. (format #f (G_ "invalid description: ~s") description) Given that you'd be trying to get the translation for "invalid description: guile" for example, I'm not sure you can defer the translation without also defering customising the message, if that makes sense? I haven't actually tried this yet, so I could be wrong. >> +(define (append-warnings . args) >> + (fold (lambda (arg warnings) >> + (cond >> + ((list? arg) >> + (append warnings >> + (filter lint-warning? >> + arg))) >> + ((lint-warning? arg) >> + (append warnings >> + (list arg))) >> + (else warnings))) >> + '() >> + args)) > > I always feel that we should have procedures that operate on lists of > anything, like =E2=80=98append=E2=80=99, and thus =E2=80=98append-warning= s=E2=80=99 looks like an > anti-pattern to me. > > What about simply ensuring that every checker returns a list of > s? That way, we wouldn=E2=80=99t have to do such things, I= think. I did consider that initially, but it involved restructuring the code even more, so I put it off. In this latest patch though, I have adjusted it so all the checkers return lists of warnings. Thanks for taking a look :) Chris --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlzyzU5fFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE 9XfciA/+PintFk8rgFxv4dP9J8jJzehnYC7/cGNpGnfAf3JPN9uXmRRQlgPUJcvK Q1VP4fCqZVsLRWgRkjuLMseg58GRjCs/kXHp/6UX6uzfHFc+j75O5HDRfytK/ZpN 1yDq8rRogI+53HYJquVP2e1d25pWaqihWtHMt07ZKcAISNC6Shw0/lMKntI49yp+ pOwTBwuiysZLjpM5O6Gfbc0t2Slk7VW13t/c5WQp1Cs+ZEvKMlsCo5ra/gPF/YT3 YS0KoHATV0Ng4Qf92PzOsuXCnYDiAYQZqD3H7UNpL0nC3UbP6v4jZJ1t7FyXUHOV qfaO0XwStTa5+pdnz5W2iQPziyFVxL9JDMkpX+xSOwKrDFt1uDX0fBU/FtOapJiJ eIIrDdsHt66i3Ozz111OnLq7pPpeDcdKbW3E+hL9/WJ/OK/JxleXIx/ZEq32eECT DXEzg/HGZeD/ipCmFId5L58NnTxCwqy/fnrOl5pHmwixKA9HkuOis82ILJgdRACu Jd8TGxnyocJdQ6PqGX9nD9XHA7ecTJqN+wc5ujmA8FMJDN74pkasXqgzNEj3yAkg Lc3fobbL7qMYNMn+WKi/MqFlp5jlzukUHJtnc2V7GSX9NkpjuUG6FOiSnqD1ZB1Q Cn+nDVY6j/+aJpmE0XfiuArkqaNpWsUYTFmhvWUIlPz+XOXj51s= =j76C -----END PGP SIGNATURE----- --=-=-=--