-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support literals encoding conversions according to the literal type. #162
base: gcos4gnucobol-3.x
Are you sure you want to change the base?
Support literals encoding conversions according to the literal type. #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while the overall approach is fine (misses documentation and tests for utf8, but both should come after the new command line argument) there are some things to adjust as detailed below; please try to get to those until Tuesday and push when finished
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice next commits; testcase for that feature and Changelog for the last commits seems too be missin
In this case (after my push) opening the file will create an error if that was really the encoding, and iconv will return an error on first use if the given encoding does not match the input.
|
2024-07-06 Ahmed Maher <ahmed.maher.ahmed.309@gmail.com> | ||
|
||
* tree.c: added encoding conversion support to cb_build_national_literal and | ||
cb_build_alphanumeric_literal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent by two spaces
@AhmedMaher309: I've fixed the merge conflict for the Changelog so the CI can run here; note that I've also applied the Changelog format to your entries - please have a look to get an idea how this (quite commonly, especially in GNU space) is done; you may also want to check the GNU coding guide on this for a better understanding I'm looking forward for your documentation entry on the encoding topic, even when we can't consider it for the final evaluation (because of the deadline now being over) [note: I plan to do the evaluation part until Thursday - it's just some quite busy week so far) |
For the failing CI build - you may want to use { } around that :-) |
I know that's a very late thought (post GSOC project timeline)... But we definitely should add the option to set |
You haven't seem to pushed your last changes, do you? |
5a2a864
to
7efa05c
Compare
…Windows and Macos Co-authored-by: Fabrice Le Fessant <fabrice.le_fessant@ocamlpro.com>
…bol-3.x` Workflow setup shall be retargetted to branch `gnucobol-3.x` upon merge
to prevent this to be caught late, as happened with OCamlPro#85
…s_literal in one function cb_build_literal_by_category
cobc: * cobc.c, codegen.c, tree.c: fixed C89 errors * typeck.c (trimmed_size, is_blank): minor refactorings
* cobc.c, codegen.c, tree.c: fixed C89 errors * typeck.c (trimmed_size, is_blank): minor refactorings * typeck.c: provide the original buffer if encoding cannot be applied run_functions.at: testsuite drafts (currently failing) for encoding related issues testsuite environment update tests: * atlocal.in (set_utf8_locale): new function for the testsuite enabling tests to either run with UTF8 locale or skip * atlocal.in: use configure-setup for the grep binary * atlocal_win: updated to current atlocal.in [to be used for encoding and possibly screenio tests]
c291c69
to
b95e404
Compare
cobc/cobc.h
Outdated
@@ -214,6 +221,7 @@ enum cb_std_def { | |||
CB_STD_BS2000, | |||
CB_STD_ACU, | |||
CB_STD_RM, | |||
CB_STD_GCOS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop this line...
tests/testsuite.src/configuration.at
Outdated
@@ -331,7 +331,7 @@ AT_CHECK([$COMPILE_ONLY \ | |||
-fstandard-define=99 prog.cob], [1], [], | |||
[configuration error: | |||
-fstandard-define=99: invalid value '99' for configuration tag 'standard-define'; | |||
maximum value: 9 | |||
maximum value: 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and that line
Added the support for encoding conversion for National literals to be in UTF-16 and for alphanumeric literals to be in ISO-8859-15 and for the UTF-8 literals to be in UTF-8
Changes in details: