RagnarGrootKoerkamp / BAPCtools

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

Latex samples #376

Closed mzuenni closed 4 months ago

mzuenni commented 4 months ago

Fixes #373 This removes the old notes section, but that one was out of spec and just shortly available so it should be no problem to just remove it again

mzuenni commented 4 months ago

do we want to (re)add an environment notes which should be used to add notes for samples?

RagnarGrootKoerkamp commented 4 months ago

I don't think a notes section is needed anymore.

Code lgtm.

Can you explain why the fallback samples are needed? Since this only adds things (apart from removing unused notes section), shouldn't it just work for existing packages?

mpsijm commented 4 months ago

Regarding the Notes section: I don't think that's necessary, you can simply write \subsection*{Notes} instead.

Regarding the fallback samples: I don't think any contest overrides bapc.cls, so I guess the fallback samples aren't needed?

mzuenni commented 4 months ago

the fallback is needed if you overwrite the problem.tex or contest-problem.tex and the document class which is done for GCPC and NWERC.

The old workflow was to first include the statement and then include the samples.tex. The samples.tex would directly print all problems.

The new workflow is to first include the samples.tex but it does not print anything (except for the fallback). It only defines commands like \Sample1{} which are needed to make the samples available in the problem statement. (See the diff in problem.tex or contest-problem.tex)

(Note that you cant call \Sample1 directly but need to call \nextsample)

mpsijm commented 4 months ago

But, for every sample, you call fallback_call.append(f'\t\\csname Sample{i+1}\\endcsname\n'). If the overridden problem.tex (or contest-problem.tex) forgets to include samples.tex, then \SampleX also is not available :thinking:

Just to make sure we're talking about the same piece of code, I'm talking about this part:

    # this is only for backwards compatibility
    samples_data += [
        '% this is only for backwards compatibility\n',
        '\\ifcsname remainingsamples\\endcsname\\else\n',
        ''.join(fallback_call),
        '\\fi\n',
    ]

Which I interpret as "if \remaningsamples is not defined, print a list of \SampleX". But \remainingsamples is always defined in bapc.cls. And if users do use a different .cls file, they'll have to implement their own sample-printing anyway. Since we also do this for NWERC, I just tried removing the fallback_call, and the samples are still in the PDF :slightly_smiling_face:

mzuenni commented 4 months ago

yes, we are speaking about the same code.

And if users do use a different .cls file, they'll have to implement their own sample-printing anyway.

No? you could use sample.tex and your own problem.cls. (In fact, that's what we are doing for gcpc now because it's easier). And since this is possible and works fine we should include this fallback for backwards compatibility.

RagnarGrootKoerkamp commented 4 months ago

ok all sgtm. Maybe just add some comments in the code explaining these details? (We could even add such usage comments in the generated .tex file but that's not all that useful probably.)

Should we also document this somewhere (probably not since it's already in the spec)? Or at least add/modify some examples for it? In that case probably modify the statement for the identity problem to do \nextsample twice and then \remainingsamples or so

mzuenni commented 4 months ago

Can we merge?