Print with overflow multiline #248
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## gcos4gnucobol-3.x #248 +/- ##
=====================================================
- Coverage 67.36% 67.11% -0.25%
=====================================================
Files 34 34
Lines 61340 61442 +102
Branches 16000 16019 +19
=====================================================
- Hits 41321 41237 -84
- Misses 14119 14294 +175
- Partials 5900 5911 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I haven't checked the details yet, but if the missing change-coverage is correct - can you do anything about that? |
I will have a look. |
There was a problem hiding this comment.
looks very good; ideally you can add something where the error prefix is set (instead of " " only) and there's a line break, but more than one will likely be hard to get (maybe with something that includes a long paragraph name defined in a long section name?)
In any case please add an entry to cobc/Changelog (we only add test Changelogs for non testsuite.src or refactoring in those, so that isn't needed)
Seems that I'm able to upstream that directly afterwards.
[Note: I commonly do that for the first commits and offer direct write access afterwards]
cobc/cobc.c
Outdated
| /* build continuation prefix */ | ||
| memset (print_data, ' ', prefix_len - 1); | ||
| out = print_data + prefix_len - 2; | ||
| allowed = (unsigned int)(max_chars_on_line - (prefix_len - 1)); |
There was a problem hiding this comment.
This line may be done before the loop, no?
If it is then out and content_cap can be set directly in the declaration.
There was a problem hiding this comment.
In commit 74830459 I have modified the code as suggested.
tests/testsuite.src/listings.at
Outdated
|
|
||
| command line: | ||
| cobc -fttitle=GnuCOBOL_V.R.P -fno-ttimestamp -q -std=default -Wall | ||
| + -fno-tmessages -fsyntax-only -t- -fno-tsymbols -ftcmd |
There was a problem hiding this comment.
The second line is a directly needed wrap, maybe you can reorder so the third line is a directly non-needed wrap? Most likely you can specify -t as often as you want and for the first time use a target name that leads to the wrap on "-t-" (just place the two behind up front)
There was a problem hiding this comment.
Simon, I didn't really understand what you meant by a directly non-needed wrap.
If I add an extra -t and move the last two arguments before the -t it looks like this:
command line:
cobc -fttitle=GnuCOBOL_V.R.P -fno-ttimestamp -q -std=default -Wall
+ -fno-tmessages -fsyntax-only -fno-tsymbols -ftcmd
+ -tcompiler_listing_file_with_long_name.txt -t- -Wimplicit-define
+ -Wstrict-typing multiline_errmsg.cob
but I'm not sure that's what you meant.
There was a problem hiding this comment.
I've meant somehting like
GnuCOBOL V.R.P prog.cob Page 0002
command line:
cobc -fttitle=GnuCOBOL_V.R.P -fno-ttimestamp -q -std=default -Wall
+ -fno-tmessages -fsyntax-only -fno-tsymbols -fdiagnostics-show-option -t abcde
+ -ftcmd -fdiagnostics-show-line-numbers -fdiagnostics-absolute-paths -t-
+ -fno-remove-unreachable prog.cob
-fno-tmessagesis exactly too much and brings a line breakabcdeis exactly enough to fit
There was a problem hiding this comment.
In commit d3b3bae I amended the command line test in listings.at as you suggested, I then discovered that print_with_overflow was breaking one column too early so I fixed that in the same commit.
|
@GitMensch Simon, it looks to me as if the lines that the coverage tool flagged are "belt and braces" checks to avoid buffer overflows. For example one of the lines checks that "prefix" is not longer than the line length, but since the line length is always either 80 or 120, and the function is only ever called with "prefix" set to 2 spaces, or "error" or "warning", there is no way to force cobc to execute these lines. Possibly these coverage alerts may disappear after I have addressed your code review comments. The coverage tool has also flagged files that weren't changed by this branch, not sure why. |
|
For code that really is unlikely to happen (in this case there would have to be a broken message catalogue creating a too long text for the prefix) and not easy to test you can use markers like in the following example: /* LCOV_EXCL_START */
if (content_size >= COB_NORMAL_BUFF) {
/* in unlikely case of "cob_core_filename is too long" use the simple one */
sprintf (cmd, "gcore -a -o %s %d", default_name, cob_sys_getpid ());
}
/* LCOV_EXCL_STOP */ |
There are already some tests in listings.at AT_SETUP([Invalid PICTURE strings]) which produce multiline error messages: I haven't succeeded in finding a way to produce an error or warning which spills over onto a third line. I did manage to produce a two-line warning: but I don't know if it is worth adding this as a new test, bearing in mind there are already two-line error tests in the testsuite? |
|
You can have this in either the same program or another one (multiple AT_TEST in one test are fine), that helps a bit for specific testing.
The one message I saw yesterday was something like "must be defined in WORKING-STORAGE, LOCAL-STORAGE or ...", that's a bit longer but may not wrap to the third line.
For the other question: I will respond when I'm at the PC, possibly tomorrow.
Roger Bowler ***@***.***> schrieb am Samstag, 18. Oktober 2025 um 12:37:
… rbowler left a comment (OCamlPro/gnucobol#248)
@GitMensch
> ideally you can add something where the error prefix is set (instead of " " only) and there's a line break,
There are already some tests in listings.at AT_SETUP([Invalid PICTURE strings]) which produce multiline error messages:
autofonce run -id 131
000037 03 PIC +(5)--.
error: only one of the symbols '-' , '+' or '$' may occur multiple times in a
+ PICTURE string
000038 03 PIC $(4)Z(9).
error: a Z or * which is before the decimal point cannot follow a floating
+ currency symbol string which is before the decimal point
I haven't succeeded in finding a way to produce an error or warning which spills over onto a third line.
I did manage to produce a two-line warning:
000005 FILE-CONTROL.
000006 SELECT TESTFILE
000007 ASSIGN TO a-very-long-name-which-has-not-been-defined-at-all
000008 ACCESS IS an-invalid-name.
...
000016 PROCEDURE DIVISION.
warning: variable 'a-very-long-name-which-has-not-been-defined-at-all' will be
+ implicitly defined
but I don't know if it is worth adding this as a new test, bearing in mind there are already two-line error tests in the testsuite?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Bingo! Good find! That message can be made to wrap onto 3 lines if the item name is long enough. And the three line error can be made into a three-line warning instead by -frelax-syntax-checks but it would require two compilations to test both error and warning, which might be a bit of an overkill. So I will add one new test containing a 2-line warning and a 3-line error, unless you think there should be a test case for a 3-line warning too. |
|
In commit ec2b536 I have added the new test for multiline error/warning messages. |
There was a problem hiding this comment.
the C89 break (should be visible in the CI result, but that's broken for other reasons currently but fixed soon with inclusion of #244 in the GH branch) needs to be fixed
the unsigned >0 is "picky" by me and may be seen more as a design "wish"
hooray for the new message test; for the line break test see my notes
tests/testsuite.src/listings.at
Outdated
|
|
||
| command line: | ||
| cobc -fttitle=GnuCOBOL_V.R.P -fno-ttimestamp -q -std=default -Wall | ||
| + -fno-tmessages -fsyntax-only -t- -fno-tsymbols -ftcmd |
There was a problem hiding this comment.
I've meant somehting like
GnuCOBOL V.R.P prog.cob Page 0002
command line:
cobc -fttitle=GnuCOBOL_V.R.P -fno-ttimestamp -q -std=default -Wall
+ -fno-tmessages -fsyntax-only -fno-tsymbols -fdiagnostics-show-option -t abcde
+ -ftcmd -fdiagnostics-show-line-numbers -fdiagnostics-absolute-paths -t-
+ -fno-remove-unreachable prog.cob
-fno-tmessagesis exactly too much and brings a line breakabcdeis exactly enough to fit
|
@GitMensch I have now received the confirmation from the FSF. |
3 participants
Print with Overflow Multiline
Summary
This pull request introduces an enhancement to the -ftcmd functionality of cobc. The changes ensure that when the cobc command line exceeds the page width of the compiler listing, it is properly wrapped and split across as many continuation lines as are necessary.
Changes Included
Motivation
The previous implementation printed the cobc command line with a maximum of one continuation line. The rest of the command line was truncated, as in this example:
This update addresses that issue, producing output like this:
Impact
Testing
Please review and provide feedback or suggestions.