Improve jparse error messages in yyerror(). #1049
Conversation
In particular, instead of surrounding the bad token with <>s, which could be one of the invalid chars and which might make someone think that that's the problematic character, do not surround it any more. Since it is by itself after the ':' (and the next char is the newline) it should be clearer this way. Updated JPARSE_VERSION and JPARSE_LIBRARY_VERSION to "1.2.8 2024-12-31" and "2.2.3 2024-12-31" respectively. NOTE: there is ABSOLUTELY NO functional change in this, just a display improvement, and so it's a useful change to include here too.
|
I aim to look at the issue about submit server soon ... then when I can get that in maybe (unless you discover something else?) we can have another code freeze before the contest opens? Well we'll see I guess. |
|
BTW: I don't think it will affect us (though I didn't look in great detail .. though it does seem to affect a lot of people anyway) but I got this warning in workflow in jparse when I forgot to update the error location files (to make sure error location testing works, if you recall):
We'll see but thought you might want to know! |
|
And I see a failed check .. hmm .. fixing this first. |
|
Apparently this repo needs a rule to rebuild error location files too .. I forgot we have additional error files here! Will fix. |
|
Display problem .. trying to fix with my bleary eyes. The rule already existed but needed to be updated. Unfortunately it looks funny when running from this repo ... there might be another way to handle it though. After this is solved I hope to look at the other issue, the top priority one .. this one seemed pretty useful though and since I discovered it I wanted to fix it before I forgot. |
|
.. discovered another potential problem .. will require a commit to jparse and to this repo too. The display issue I have an idea of what it might be but it only happens here and only from the top level Makefile rule. I guess it's not worth trying to figure out for now as only we would run it and more than likely only I would run it as it only has to be done if and only if jparse error messages changed. And that is extremely rare and it's very unlikely it'll change again: if it does it'll not be for a long time I guess. |
|
Oh I found the problem ... it was a typo in the Makefile in jparse/test_jparse! I'll fix that along with the other change and then merge it here and do the fix as well .. so this should be fine soon. |
Fix typo in jparse rebuild_jparse_err_files rule that was causing a display problem. Also here the -v flag cannot depend on the VERBOSITY level as the script was designed specifically for -v 0! This has been fixed. The rebuild_jparse_err_files rule HERE must also use the new rule in test_ioccc/Makefile as we have additional error location files to test. Although only Landon or I (Cody) will ever need to do this and it's also extremely unlikely it'll have to happen again, it is still a needed update. This commit should fix the workflow error in the previous commit 33951ba.
|
The commit I just made should theoretically solve the problems. I need to focus my eyes and then I can hopefully look at your comment at the other issue and resolve the two top priority issues, hopefully allowing for a code freeze! Then after that I can look at the issues in the winner repo! |
|
Oops .. let's try again! |
|
Oh bugger me .... the problem is the path to jparse in the error files and to do with the way jparse(1) will show the program name. Since I can put party.json in jparse/test_jparse/test_JSON/bad_loc (via jparse repo) I can get rid of the directory here as that's the only file. Since |
|
Seems I got it .. have to do a commit over there and then I have to do the commit here and it should be okay! |
|
Almost there .. had to go afk a bit. Made commit at jparse repo ... now recreating clone here and then will do make all test and if all is good I'll commit. |
This had to be done due to the improvement with yyerror() and how the error messages have to be exact. It might be possible to fix the issue but it is not worth it as long as jparse here tests location errors and it now does. Plus party.json belongs in jparse anyway. What this commit means is that the files were moved from test_ioccc/test_JSON/general.json/bad_loc to jparse/test_jparse/test_JSON/bad_loc. The top level Makefile and the test_ioccc/Makefile both had to be updated but that's only because we no longer need the error location files in test_ioccc/test_JSON/general.json/bad_loc so the relevant rule no longer calls the no longer existing rule rebuild_jparse_err_files. Updated test_ioccc/ioccc_test.sh to use option -L in jparse_test.sh as we no longer want to test error location messages. This is the only functional difference in this commit. Updated ioccc_test.sh version to: "1.0.3 2024-12-31".
|
This should do it. Only one minor functional difference here and the benefit is better error reporting in jparse. I'll next look at the other issue ... though not sure if that's right away or not. |
Happy New Year to you too! Whatever else might or might not go: at least we're at the point where the IOCCC can run again! I'm going to try looking at the other repo but I'm not sure what I'll get done today. I'm rather tired. I do know one thing though that should be done and I believe I did it but I have to verify it. Anyway that's for over there. Does this solve the two top priority issues? |
Very close, @xexyl. See our suggestion in GH-issuecomment-2566672002. With that we think we can release a new mkiocccentry repo. BTW: We suggest that you call that repo version 2.2. UPDATE 0Correction: repo version 2.3 |
In particular, instead of surrounding the bad token with <>s, which could be one of the invalid chars and which might make someone think that that's the problematic character, do not surround it any more. Since it is by itself after the ':' (and the next char is the newline) it should be clearer this way.
Updated JPARSE_VERSION and JPARSE_LIBRARY_VERSION to "1.2.8 2024-12-31" and "2.2.3 2024-12-31" respectively.
NOTE: there is ABSOLUTELY NO functional change in this, just a display improvement, and so it's a useful change to include here too.