Add a setting for the default tabstop width.

Review Request #14361 — Created March 4, 2025 and submitted — Latest diff uploaded

Information

Review Board
release-7.x
839

Reviewers

One frequent request is to have the ability to show tabstops as
something other than 8 characters. When we first started getting this
request, the tab-size CSS rule was very poorly supported, and so there
wasn't much we could do. Many years later, it's been properly
implemented in all the browsers we care about, so it's well past time to
support this use case.

This change lays the framework for doing this, and implements the bare
minimum case of setting a global default for the tabstop size. This
involves several pieces:

  • A new extra dict has been added to the diff file context and plumbed
    through the diff viewer, views, and JS model.
  • A new setting for the default has been added to siteconfig and the
    DiffSettings object. The settings has been plumbed through a few
    more places in order to get it where it needs to go for this.
  • Plumb the diff settings/tab size through into the chunk and opcode
    generators, so that they will take it into account (for things like
    indentation change markers).
  • The DiffReviewableView will check if there's a tab_size key in the
    extra object, and use that to set the CSS for the file.

A natural future enhancement to this is to add an extension hook that
can modify the DiffSettings for an individual file, which will allow us
to build an extension that could do things like use .editorconfig files
in order to manage this on a per-path or per-file-type basis. It might
also be nice to give the user some kind of UI for changing the displayed
tabstop width for a file on the client side.

While in here, this fixes up a typo in the diff settings form, and the
diff settings form documentation.

  • Set the tabstop setting to various options (empty, 0, 4, 8, 12) and
    saw that it was properly reflected in the diff viewer for a change
    involving tabs.
  • Tested that indentation change markers worked correctly with different
    tabstop settings, including 1 and 2 characters (where the marker is
    truncated to fit in the space).
  • Ran unit tests.

Changes between revision 3 and 4

orig
1
2
3
4
5

Commits

Summary ID Author
Add a setting for the default tabstop width.
One frequent request is to have the ability to show tabstops as something other than 8 characters. When we first started getting this request, the `tab-size` CSS rule was very poorly supported, and so there wasn't much we could do. Many years later, it's been properly implemented in all the browsers we care about, so it's well past time to support this use case. This change lays the framework for doing this, and implements the bare minimum case of setting a global default for the tabstop size. This involves several pieces: - A new `extra` dict has been added to the diff file context and plumbed through the diff viewer, views, and JS model. - A new setting for the default has been added to siteconfig and the `DiffSettings` object. The settings has been plumbed through a few more places in order to get it where it needs to go for this. - Plumb the diff settings/tab size through into the chunk and opcode generators, so that they will take it into account (for things like indentation change markers). - The `DiffReviewableView` will check if there's a `tab_size` key in the `extra` object, and use that to set the CSS for the file. A natural future enhancement to this is to add an extension hook that can modify the DiffSettings for an individual file, which will allow us to build an extension that could do things like use `.editorconfig` files in order to manage this on a per-path or per-file-type basis. It might also be nice to give the user some kind of UI for changing the displayed tabstop width for a file on the client side. While in here, this fixes up a typo in the diff settings form, and the diff settings form documentation. Testing Done: - Set the tabstop setting to various options (empty, 0, 4, 8, 12) and saw that it was properly reflected in the diff viewer for a change involving tabs. - Tested that indentation change markers worked correctly with different tabstop settings, including 1 and 2 characters (where the marker is truncated to fit in the space). - Ran unit tests. Addresses bug 839.
535fcf86c02f100f5609458c6ad289f3a9f27c98 David Trowbridge
Add a setting for the default tabstop width.
One frequent request is to have the ability to show tabstops as something other than 8 characters. When we first started getting this request, the `tab-size` CSS rule was very poorly supported, and so there wasn't much we could do. Many years later, it's been properly implemented in all the browsers we care about, so it's well past time to support this use case. This change lays the framework for doing this, and implements the bare minimum case of setting a global default for the tabstop size. This involves several pieces: - A new `extra` dict has been added to the diff file context and plumbed through the diff viewer, views, and JS model. - A new setting for the default has been added to siteconfig and the `DiffSettings` object. The settings has been plumbed through a few more places in order to get it where it needs to go for this. - Plumb the diff settings/tab size through into the chunk and opcode generators, so that they will take it into account (for things like indentation change markers). - The `DiffReviewableView` will check if there's a `tab_size` key in the `extra` object, and use that to set the CSS for the file. A natural future enhancement to this is to add an extension hook that can modify the DiffSettings for an individual file, which will allow us to build an extension that could do things like use `.editorconfig` files in order to manage this on a per-path or per-file-type basis. It might also be nice to give the user some kind of UI for changing the displayed tabstop width for a file on the client side. While in here, this fixes up a typo in the diff settings form, and the diff settings form documentation. Testing Done: - Set the tabstop setting to various options (empty, 0, 4, 8, 12) and saw that it was properly reflected in the diff viewer for a change involving tabs. - Tested that indentation change markers worked correctly with different tabstop settings, including 1 and 2 characters (where the marker is truncated to fit in the space). - Ran unit tests. Addresses bug 839.
6ba793eebd4a46bf39d300ac61de7cc723e31888 David Trowbridge
docs/manual/admin/configuration/diffviewer-settings.rst
reviewboard/diffviewer/chunk_generator.py
reviewboard/diffviewer/diffutils.py
reviewboard/diffviewer/opcode_generator.py
reviewboard/diffviewer/settings.py
reviewboard/diffviewer/views.py
Loading...