From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by sourceware.org (Postfix) with ESMTPS id 752CA387605F for ; Mon, 16 Mar 2020 16:10:33 +0000 (GMT) X-Originating-IP: 91.166.131.130 Received: from localhost (91-166-131-130.subs.proxad.net [91.166.131.130]) (Authenticated sender: dodji@seketeli.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 455601BF213; Mon, 16 Mar 2020 16:10:16 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 81908581890; Mon, 16 Mar 2020 17:10:15 +0100 (CET) From: Dodji Seketeli To: Giuliano Procida Cc: libabigail@sourceware.org, kernel-team@android.com Subject: Re: [PATCH 3/5 v2] [abidiff] Eliminate some unnecessary blank lines in diff output. Organization: Me, myself and I References: <20200312063036.29419-4-gprocida@google.com> <20200316131025.261522-1-gprocida@google.com> X-Operating-System: Fedora 33 X-URL: http://www.seketeli.net/~dodji Date: Mon, 16 Mar 2020 17:10:15 +0100 In-Reply-To: <20200316131025.261522-1-gprocida@google.com> (Giuliano Procida's message of "Mon, 16 Mar 2020 13:10:25 +0000") Message-ID: <87y2s0uvq0.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-0.2 required=5.0 tests=JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Mar 2020 16:10:34 -0000 Hello Giuliano, Giuliano Procida a =C3=A9crit: > v2: More code simplification. Tests unchanged. > > There is distributed responsibility for horizontal and vertical > whitespace in the reporting code with indent and new line control > information being manipulated by and passed in and out of functions. > Occasionally, this information is ignored or incorrect and the code > tends to err on the side of more rather than fewer new lines. > > The outcome is that abidiff output sometimes contains extra blank > lines which can be confusing. > > This patch eliminates some of the more obvious cases: > > - after data member deletions > - in enumerator change lists > - after "type size hasn't changed" > - before listing impacted interfaces > > A lot of passing of "new line needed" booleans between functions has > been eliminated in the process. > > The patch cleans up the reporting of data members. The code will > either emit indentation, a short description and a new line or do > nothing at all. > > The patch also removes some stray location reporting code for array > diffs which would have produced some oddly placed output. I could not > get this code to trigger as loc =3D decl->get_location() was never > present on the array decls in question. > > * src/abg-default-reporter.cc (report): In the type_decl_diff, > enum_diff, array_diff, class_diff, union_diff and var_diff > overrides, simplify new line logic which no longer needs to be > threaded through report_name_size_and_alignment_changes. In > the distinct_diff override, simplify new line logic which no > longer needs to be threaded through > report_size_and_alignment_changes. In the enum_diff override, > emit just one blank line after each enum. In the array_diff > override, remove stray location reporting which doesn't appear > to ever trigger; fix new line logic. In the > class_or_union_diff override, simplify new line logic for > deleted members; pass indentation to represent_data_member. > * src/abg-leaf-reporter.cc (report): In the array_diff, > class_diff, union_diff and var_diff overrides, simplify new > line logic which no longer needs to be threaded through > report_name_size_and_alignment_changes. In the distinct_diff > override, simplify new line logic which no longer needs to be > threaded through report_size_and_alignment_changes. In the > array_diff override, remove stray location reporting which > doesn't appear to ever trigger; fix new line handling. In the > class_or_union_diff override, simplify new line logic for > deleted members; pass indentation to represent_data_member. > In the corpus_diff override, tabify source indentation. > * src/abg-reporter-priv.cc (represent_data_member): Handle > indentation; fix new line logic. > (report_size_and_alignment_changes): Fix new line logic > for "type size hasn't changed" message; simplify new line > logic and replace local bool n with argument bool nl for > clarity. > (report_size_and_alignment_changes): Remove bool nl argument > and associated code as it had become always false; take > responsibility for emitting terminating new lines and change > return type to void. > (report_name_size_and_alignment_changes): Fix new line logic; > remove bool nl argument and associated code as it had become > always false; take responsibility for emitting terminating new > lines and change return type to void. > (maybe_report_interfaces_impacted_by_diff) In both overrides, > remove new line prefix code and new_line_prefix argument. > * src/abg-reporter-priv.h (represent_data_member): Add indent > argument. > (report_size_and_alignment_changes) Remove bool nl argument; > change return type to void. > (report_name_size_and_alignment_changes) Remove bool nl > argument; change return type to void. > (maybe_report_interfaces_impacted_by_diff) In both overrides, > remove new_line_prefix argument. > * tests/data/test-*/*report*.txt: Remove some blank lines. Whoah, thanks a lot for this great work. And many thanks to Matthias for the review. What can I say other than, Well, applied to master. Thanks! --=20 Dodji