RagnarGrootKoerkamp / BAPCtools

Tools for developing ICPC-style programming contest problems.
GNU General Public License v3.0
49 stars 22 forks source link

326 warn if problemtitle differs from problemsettingsnamelanguage #329

Closed thorehusfeldt closed 8 months ago

thorehusfeldt commented 8 months ago

For #326, compare problemtitle in problem.yaml and various {lang}.tex files.

The extraction of \problemname{foo} in problem.en.tex is handled by a new method in bin/latex.py. This is very bare-bones and would be confused by each of the following

% \problenname{No warning produced, though outcommented}
\problemname{
   foo
}

Somebody should run this on a large repository of existing problems and see how many false positive warnings it generates.

It warns if no problemname is found in the tex source, which makes the BAPCtools testsuite unhappy. Maybe that warning shouldn’t be there. On the other hand, the specification actually requires it:

https://github.com/Kattis/problem-package-format/blob/af5288b5c21d8a65ace08296ce7aecad8fdefdd2/spec/2023-07-draft.md?plain=1#L282

(and in the first line of the code!) so maybe it should be much stricter.

thorehusfeldt commented 8 months ago

I’ve now implemented the substring matching; looks like this:

             texname_letters = iter(texname)
                if not all(c in texname_letters for c in yamlname):
                    warn(f'Problem titles in problem.{lang}.tex ({texname})' +
                         f' and problem.yaml ({yamlname}) differ;' +
                         r' consider using \problemname{\problemyamlname}.'
                         )

Before I commit: Is this really what you want? It would accept

\problemname{A Slightly Different Problem}

when problem.yaml is

problem: { en: A Different Problem }

and similar situations, like https://open.kattis.com/problems/rationalsequence2 (which would not catch problem: { en: A Rational Sequence}. My own hunch is that markup in TeX problemtitles is less common than the above. I could be wrong.

RagnarGrootKoerkamp commented 8 months ago

For us we pretty much always use \problemyamlname unless the formatting is different, but maybe you're right.

We could also allow some way to disable the check, like \problemname{something different} % DISTINCTNAME. Then the check can be strict and users can simply disable the warning.

@mpsijm @mzuenni wdyt?

thorehusfeldt commented 8 months ago

Another idea is to disable the check as soon as texname contains \.

RagnarGrootKoerkamp commented 8 months ago

Oh, I like that! Feel free to implement.

thorehusfeldt commented 8 months ago

Can someone grep \\problemname\{^\\}(or something like that) in the full BAPC- and NWERC-library? Then we could get a better feeling for what kind of formatting actually appears in real life.

RagnarGrootKoerkamp commented 8 months ago

For BAPC we have:

For NWERC we didn't deviate in the past 3 years.

thorehusfeldt commented 8 months ago

I did toy with the idea of removing matching \foo{} from \problemtitle but I don’t think this issue deserves that much attention. I think I’m done with my TODOs now.

Do note that 62a41b1 escalated warn to error, which is consistent with the specification (and now also with BAPCtools’s test suite.)

RagnarGrootKoerkamp commented 8 months ago

I did toy with the idea of removing matching \foo{} from \problemtitle but I don’t think this issue deserves that much attention.

Agreed.

I think I’m done with my TODOs now.

Do note that 62a41b1 escalated warn to error, which is consistent with the specification (and now also with BAPCtools’s test suite.)

:+1:

Thanks! Merging :)