Re: [PATCH 3/5 v2] [abidiff] Eliminate some unnecessary blank lines in diff output.

[Dodji Seketeli <dodji@seketeli.org>]

Hello Giuliano,

Giuliano Procida <gprocida@google.com> a écrit:

> 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 = 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!

-- 
		Dodji