Add a setting for the default tabstop width.
Review Request #14361 — Created March 4, 2025 and submitted
One frequent request is to have the ability to show tabstops as
something other than 8 characters. When we first started getting this
request, thetab-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 atab_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.
Summary | ID |
---|---|
a46adda8039b221464322c4c7749a42b45a43acf |
Description | From | Last Updated | ||
---|---|---|---|---|
We should have documentation for this new setting in the /admin/configuration/diffviewer-settings/ docs page. |
![]() |
|||
I think it'd be good to add a DiffSettingsTests unit test where you set the tab size to 0, and … |
![]() |
|||
continuation line missing indentation or outdented Column: 5 Error code: E122 |
![]() |
|||
missing whitespace around parameter equals Column: 51 Error code: E252 |
![]() |
|||
missing whitespace around parameter equals Column: 52 Error code: E252 |
![]() |
|||
I feel like we should set this default in reviewboard.admin.siteconfig. Add a diffviewer_default_tab_size entry there and set it to DiffOpcodeGenerator.TAB_SIZE, … |
![]() |
|||
If we do set the default tab size in siteconfig, then we probably don't need to do this here? And … |
![]() |
|||
Can you add a Version Added here. |
![]() |
|||
Can you add a Version Added here. |
![]() |
|||
Should be two blank lines between content and reference definitions. |
|
|||
I think we can just make implementations plural. Also, missing a trailing period. |
|
|||
Should probably use Sequence instead of list. |
|
|||
Sequence here as well. |
|
|||
Can you update this to use the standard typed function indentation form? |
|
|||
I think this is missing Version Added. |
|
|||
These should be the same import group. |
|
|||
Thinking DiffSettings should define a DEFAULT_TAB_SIZE, and then we can reference this here and assign the others to alias that. … |
|
|||
This could be one statement now. |
|
|||
Should this be Mapping? Unless we mutate it later. |
|
|||
We're missing **kwargs in the docs here. |
|
|||
Might as well cut out the middle man and change this to DiffSettings.DEFAULT_TAB_SIZE now. |
![]() |
|||
Same here and below. |
![]() |
|||
There are no open issues |
- Commits:
-
Summary ID 9c5e5513aaadb2b2aa94270d4c8a50cce1557419 306873257031931119f3738b46d9c277408ef4ef - Diff:
-
Revision 2 (+942 -230)
Checks run (2 succeeded)

-
-
We should have documentation for this new setting in the
/admin/configuration/diffviewer-settings/
docs page. -
I think it'd be good to add a
DiffSettingsTests
unit test where you set the tab size to 0, and assert that it gets changed toNone
. -
I feel like we should set this default in
reviewboard.admin.siteconfig
. Add adiffviewer_default_tab_size
entry there and set it toDiffOpcodeGenerator.TAB_SIZE
, so that we can expect there to be a default value in siteconfig if we're ever working with tab sizes outside of this class. -
If we do set the default tab size in siteconfig, then we probably don't need to do this here? And could maybe get rid of the
TAB_SIZE
attribute here altogether, so that we only have one default value to manage. -
-
- Description:
-
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 therewasn'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 atab_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
filesin 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. - A new
- Commits:
-
Summary ID 306873257031931119f3738b46d9c277408ef4ef 535fcf86c02f100f5609458c6ad289f3a9f27c98 - Diff:
-
Revision 3 (+1126 -286)
Checks run (2 succeeded)
- Commits:
-
Summary ID 535fcf86c02f100f5609458c6ad289f3a9f27c98 6ba793eebd4a46bf39d300ac61de7cc723e31888 - Diff:
-
Revision 4 (+1178 -304)
Checks run (2 succeeded)
- Commits:
-
Summary ID 6ba793eebd4a46bf39d300ac61de7cc723e31888 a46adda8039b221464322c4c7749a42b45a43acf - Diff:
-
Revision 5 (+1176 -304)