Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Refactor complex conditions in collect_tokens_trailing_token #128223

Merged
merged 4 commits into from
Jul 27, 2024
Merged

Refactor complex conditions in collect_tokens_trailing_token #128223

merged 4 commits into from
Jul 27, 2024

Commits on Jul 25, 2024

  1. Invert early exit conditions in collect_tokens_trailing_token. 

    This has been bugging me for a while. I find complex "if any of these
are true" conditions easier to think about than complex "if all of these
are true" conditions, because you can stop as soon as one is true.
    @nnethercote
    nnethercote committed Jul 25, 2024
    Configuration menu
    Copy the full SHA
    caee195 View commit details
    Browse the repository at this point in the history

  2. Inline and remove AttrWrapper::is_complete. 

    It has a single call site. This change makes the two `needs_collect`
conditions more similar to each other, and therefore easier to
understand.
    @nnethercote
    nnethercote committed Jul 25, 2024
    Configuration menu
    Copy the full SHA
    4288edb View commit details
    Browse the repository at this point in the history

  3. Move is_complete to the module that uses it. 

    And make it non-`pub`.
    @nnethercote
    nnethercote committed Jul 25, 2024
    Configuration menu
    Copy the full SHA
    3d363c3 View commit details
    Browse the repository at this point in the history

  4. Invert the sense of is_complete and rename it as needs_tokens. 

    I have always found `is_complete` an unhelpful name. The new name (and
inverted sense) fits in better with the conditions at its call sites.
    @nnethercote
    nnethercote committed Jul 25, 2024
    Configuration menu
    Copy the full SHA
    e631b1e View commit details
    Browse the repository at this point in the history