Add a central representation of settings for diff display.
Review Request #12786 — Created Jan. 13, 2023 and submitted
We have an increasing number of options that influence the display of
diffs. Most of these are site-wide, but some are influenced by user
settings (syntax highlighting in particular). Historically, while most
of these were queried during diff render, their values weren't factored
into any caching or ETags, requiring cache to be cleared for most to
take effect.As new settings are coming for diff display, to govern code safety
checkers, it was time to rethink the approach here.This change introduces
reviewboard.diffviewer.settings.DiffSettings
.
This is an option that represents all the settings that go into the
display of diffs generally. This includes things like whether syntax
highlighting is enabled, or how many diffs are on a page. It does not
include per-diff mode-specific settings, like whether to expand/collapse
a given section.The
DiffSettings
object can be computed once and then passed around to
the various parts of the diff machinery. This keeps things consistent,
and avoids having to pass around individual bits of state or to
re-compute them.It also contains a
state_hash
property, which returns a SHA256 of the
DiffSettings
state. That's now included in all cache keys and ETags.
If any of these settings change, new cache keys and ETags are used. If
they're changed back, the original cache keys and ETags are used. This
avoids stale diffs and stays performant.The old syntax highlighting flags for
DiffChunkGenerator
,
DiffRenderer
, andpopulate_diff_chunks()
have been deprecated, but
are still supported until Review Board 6.diff_settings
is now
recommended, but each object will compute based on site defaults if not
provided.This change intentionally does not add type hints for the functions that
were modified to takediff_settings
, as adding type hints for these
functions is a surprisingly-large undertaking. I'll be revisiting those
functions in the future.
Unit tests pass on all supported versions of Python.
Manually tested this with the various diff viewer settings, including
some in-progress ones for code safety. Any time I changed a setting,
reloading would give me new diffs, verifying that both cache keys and
ETags had changed. Subsequent reloads with those settings used cached
state.
- Change Summary:
-
- Added a missing import.
- Removed syntax highlighting elements of some cache/ETag keys, using the settings state hash instead.
- Commits:
-
Summary ID 7c522683a96e5731a73c2bf7233f9c02f834851e 4c4e288856d51742cd7f06c43d6605dd232935e8 - Diff:
-
Revision 2 (+1996 -238)