josephwright / beamer

A LaTeX class for producing presentations and slides
Other
1.35k stars 139 forks source link

simplify decision tree for `\beamer@howtotreatframe` #874

Closed amonakov closed 5 months ago

amonakov commented 6 months ago

Expanding the frame environment relies on one of the following macros:

Beamer used to choose between them by first branching on presence of a label, and when a label is NOT present, also by testing overlay specification to see if it yields no slides, via the following tree:

if (no label)
    if (overlay specification not empty)
        if fragile=singleslide
            \beamer@dosingleframe
        elif allowframebreaks
            \beamer@autobreakframe
        elif fragile
            \beamer@doexternalframe
        else
            \beamer@doseveralframes
        fi
    else # empty overlay specification
        \beamer@donoframe
    fi
else # has label
    if fragile
        \beamer@doexternalframe
    elif fragile=singleslide
        \beamer@dosingleframe
    else
        \beamer@doseveralframes
    fi
fi

This structure is confusing and leads to bugs such as #872 (allowframebreaks is not handled in presence of a label) and #873 (labeled fragile=singleslide frame shown despite <all:0> ospec).

It also uses \beamer@donoframe to suppress unlabeled slides, but it is inefficient (bug #866) compared to \beamer@doseveralframes which does not typeset the skipped content.

Use the following structure instead:

take \beamer@doseveralframes unless overridden below:
if allowframebreaks
    if ospec not empty
        \beamer@autobreakframe
    # else keep \beamer@doseveralframes
    fi
fi
if fragile
    \beamer@doexternalframe
fi
if fragile=singleslide
    if ospec not empty
        \beamer@dosingleframe
    else
        \beamer@donoframe
    fi
fi

Fixes #866. Fixes #872. Fixes #873.

samcarter commented 6 months ago

Thanks for your PR! As this is like an open-heart-surgery on beamer, I'll do a bit of testing before merging.

josephwright commented 6 months ago

I think this PR needs both @samcarter and I to read carefully. What we definitely should do is tidy up coding in such changes: for example, % should only be at the end of the lines where it's needed.

samcarter commented 6 months ago

There might be some difference in the handling of againframes, unfortunately the git history does not go back far enough to see why this was introduced...

josephwright commented 6 months ago

There might be some difference in the handling of againframes, unfortunately the git history does not go back far enough to see why this was introduced...

You reading Till's commits ;)

samcarter commented 6 months ago

There might be some difference in the handling of againframes, unfortunately the git history does not go back far enough to see why this was introduced...

You reading Till's commits ;)

The ones with *** empty log message *** are the most helpful ones :)

amonakov commented 5 months ago

Can you share your thoughts so far, please?

samcarter commented 5 months ago

My thoughts so far are that it seems that there is a difference how again frames are treated, but I haven't yet found an example where this causes a problem.

My proposal would be to merge this into the main branch and keep testing it until after the TL24 code freeze.

@josephwright What do you think?

amonakov commented 5 months ago

@josephwright What do you think?

Let me point out he thumbed up your response (Github doesn't send out notifications in such cases afaik).