Add flags to make listing easy to diff #105

lefessan · 2023-07-03T15:29:43Z

follow-up to the discussion originally in #75, implementing part of https://sourceforge.net/p/gnucobol/feature-requests/294/

New flags:

-fno-ttimestamp: suppress time stamp
-fttitle=<title>: display <title> instead of package name and version

lefessan · 2023-07-03T15:30:49Z

I still have many tests in listing.at to modify...

GitMensch

This could go live tomorrow - but -ftversion is really out of the question, so please adjust.

And just as a note: we don't need to add the other two to NEWS; but as this is a global change to testsuite.src/listing.at this is one of the cases where a Changelog entry for the testsuite is good.

tests/atlocal.in

tests/testsuite.src/listings.at

cobc/ChangeLog

GitMensch · 2023-07-03T15:39:35Z

cobc/cobc.c

+	if (cb_listing_with_timestamp){
+		strftime (cb_listing_date, (size_t)CB_LISTING_DATE_MAX,
+			  LISTING_TIMESTAMP_FORMAT, &current_compile_tm);
+	} else {


the idea of cb_listing_with_timestamp == 0 is to drop this completely; this provides more space (even if we only use that later) and can be a benefit to the user - while the hard values below cannot

I suggest to leave this code unchanged (= original) and instead check cb_listing_with_timestamp in print_program_header().
The current implementation would generate a trailing space for --tlines=0 which may be a reason for the testsuite failures you currently see.

cobc/cobc.c

codecov-commenter · 2023-07-03T22:26:29Z

Codecov Report

Merging #105 (afeb5ca) into gcos4gnucobol-3.x (4b8452a) will increase coverage by 0.01%.
The diff coverage is 92.85%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@                  Coverage Diff                  @@
##           gcos4gnucobol-3.x     #105      +/-   ##
=====================================================
+ Coverage              65.17%   65.19%   +0.01%     
=====================================================
  Files                     31       31              
  Lines                  58384    58393       +9     
  Branches               15380    15384       +4     
=====================================================
+ Hits                   38051    38067      +16     
+ Misses                 14402    14396       -6     
+ Partials                5931     5930       -1

Impacted Files	Coverage Δ
cobc/cobc.c	`71.53% <88.88%> (+0.25%)`	⬆️
cobc/flag.def	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

lefessan · 2023-07-03T22:37:07Z

Argh, tests run fine with autofonce but not with autotest, I need to investigate the different behavior...

GitMensch

only minor changes needed (place where cb_listing_with_timestamp is read), which may also fix the tests - as soon as this is done and the test run - feel free to commit to svn, so we can go on with the REPLACE issue

one thing that is not mandatory but may be done in this PR: move --tlines to -ftlines (drop from help.c, keep setting in cobc.c as compatibility for now, only the definition moves to flag.def)

Note: I took the freedom to adjust the help output in flags.def.

GitMensch · 2023-07-04T08:02:27Z

cobc/cobc.c

+	if (cb_listing_with_timestamp){
+		strftime (cb_listing_date, (size_t)CB_LISTING_DATE_MAX,
+			  LISTING_TIMESTAMP_FORMAT, &current_compile_tm);
+	} else {


I suggest to leave this code unchanged (= original) and instead check cb_listing_with_timestamp in print_program_header().
The current implementation would generate a trailing space for --tlines=0 which may be a reason for the testsuite failures you currently see.

cobc/cobc.c

tests/atlocal.in

GitMensch · 2023-07-04T08:23:21Z

Argh, tests run fine with autofonce but not with autotest, I need to investigate the different behavior...

Just inspected the testsuite.log: it is exactly as I've thought:

-GnuCOBOL V.R.P               prog.cob
+GnuCOBOL V.R.P               prog.cob

To fix that change the place where cb_listing_with_timestamp is checked (should be in print_program_header()) and the tests will pass. Note that this directly allows to give either the title or the filename more space, too.

lefessan · 2023-07-05T10:15:38Z

I will commit this version to SVN as soon as the CI passes, to be able to switch to the other PR.
There are still tests with spaces at the end of headers, that I fixed using a quadrigraph (@&t@) to prevent autoconf from removing them. I preferred not to change the printing because the spaces are generated by %-26.26s in printf and I was not sure of the intention.

lefessan · 2023-07-05T10:17:10Z

( I just pushed a commit in autofonce to correctly handle spaces, in case you use it)

GitMensch · 2023-07-05T10:42:14Z

I preferred not to change the printing because the spaces are generated by %-26.26s in printf and I was not sure of the intention.

The point was that there is a trailing timestamp, right adjusted.
If -fno-ttimestamp is used we currently printf the low-value; but when this is changed to not adjustt the timestamp var but instead adjust the printing and right-justify the sourcename, then we should not have any trailing spaces left.

GitMensch · 2023-07-05T14:28:47Z

LGTM (we can right-adjust the filename for -fno-ttimestamp later)

New flags: -fno-ttimestamp: suppress time stamp -fttitle=<title>: display <title> instead of package name and version Partially implements feature request #294

minor spacing issue and dropped info from help that never was true

lefessan · 2023-07-05T15:03:56Z

Merged with whitespaces removed at the end of line

This comment was marked as outdated.

Sign in to view

GitMensch requested changes Jul 3, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

lefessan force-pushed the z-2023-07-03-no-time-in-listing branch 3 times, most recently from 959c94b to ce6a132 Compare July 3, 2023 22:24

This comment was marked as outdated.

Sign in to view

GitMensch requested changes Jul 4, 2023

View reviewed changes

lefessan force-pushed the z-2023-07-03-no-time-in-listing branch 3 times, most recently from afeb5ca to 87a0695 Compare July 5, 2023 10:11

lefessan force-pushed the z-2023-07-03-no-time-in-listing branch from 87a0695 to 4654158 Compare July 5, 2023 14:15

lefessan and others added 2 commits July 5, 2023 16:32

Add flags to make listing easy to diff

a9f63bb

New flags: -fno-ttimestamp: suppress time stamp -fttitle=<title>: display <title> instead of package name and version Partially implements feature request #294

Update flag.def

766bfc1

minor spacing issue and dropped info from help that never was true

lefessan force-pushed the z-2023-07-03-no-time-in-listing branch from 4654158 to 766bfc1 Compare July 5, 2023 14:33

lefessan added the Merged in SVN label Jul 5, 2023

lefessan closed this Jul 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flags to make listing easy to diff #105

Add flags to make listing easy to diff #105

lefessan commented Jul 3, 2023 •

edited

Loading

This comment was marked as outdated.

lefessan commented Jul 3, 2023

GitMensch left a comment •

edited

Loading

GitMensch Jul 3, 2023

lefessan Jul 3, 2023

GitMensch Jul 4, 2023

This comment was marked as outdated.

codecov-commenter commented Jul 3, 2023 •

edited

Loading

This comment was marked as outdated.

lefessan commented Jul 3, 2023

GitMensch left a comment

GitMensch Jul 4, 2023

GitMensch commented Jul 4, 2023

lefessan commented Jul 5, 2023

lefessan commented Jul 5, 2023

GitMensch commented Jul 5, 2023

GitMensch commented Jul 5, 2023

lefessan commented Jul 5, 2023

Add flags to make listing easy to diff #105

Add flags to make listing easy to diff #105

Conversation

lefessan commented Jul 3, 2023 • edited Loading

This comment was marked as outdated.

lefessan commented Jul 3, 2023

GitMensch left a comment • edited Loading

Choose a reason for hiding this comment

GitMensch Jul 3, 2023

Choose a reason for hiding this comment

lefessan Jul 3, 2023

Choose a reason for hiding this comment

GitMensch Jul 4, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

codecov-commenter commented Jul 3, 2023 • edited Loading

Codecov Report

This comment was marked as outdated.

lefessan commented Jul 3, 2023

GitMensch left a comment

Choose a reason for hiding this comment

GitMensch Jul 4, 2023

Choose a reason for hiding this comment

GitMensch commented Jul 4, 2023

lefessan commented Jul 5, 2023

lefessan commented Jul 5, 2023

GitMensch commented Jul 5, 2023

GitMensch commented Jul 5, 2023

lefessan commented Jul 5, 2023

lefessan commented Jul 3, 2023 •

edited

Loading

GitMensch left a comment •

edited

Loading

codecov-commenter commented Jul 3, 2023 •

edited

Loading