Page MenuHome

Text space type: enable syntax highlighting and line numbers by default
ClosedPublic

Authored by Sybren A. Stüvel (sybren) on Tue, Aug 13, 3:36 PM.

Details

Summary

The most common use of the text editor seems to be for scripting.
Having line numbers and syntax highlighting enabled by default seems
sensible. Patch made by request of @William Reynish (billreynish). Adding @Brecht Van Lommel (brecht) as
reviewer as he made a similar change for already-existing text editors
in 84936ce.

Diff Detail

Repository
rB Blender

Event Timeline

This is a better default indeed.

One issue is that header is really very crowded - you must make it quite wide to see everything,

At the very least, I think Register can nicely go in the Text menu. It's not something you need to toggle all the time.

I think Run Script could be placed in the footer to the right:


Like an execute button.

This revision is now accepted and ready to land.Tue, Aug 13, 3:44 PM

Campbell is the maintainer of the text editor so setting him as reviewer. I'm fine with this personally, even if it will highlight in some cases where it shouldn't.

If we want to make this a bit smarter we could do the following:

  • If the name has an unknown extension (not .py, .osl, ..) , don't syntax highlight and gray out the setting in the menu.
  • If the name has no extension, still assume Python and syntax highlight. Numeric postfixes like Text.001 would not be considered to be an extension.

+1 to Brecht's comments, we'd need to update versioning_default.c too.

If we want to make this a bit smarter we could do the following:

  • If the name has an unknown extension (not .py, .osl, ..) , don't syntax highlight and gray out the setting in the menu.
  • If the name has no extension, still assume Python and syntax highlight. Numeric postfixes like Text.001 would not be considered to be an extension.

Just to check that I understand properly.

  • Press Shift+F11 in the 3D view to switch to a brand new Text editor.
  • Click the New button to create a new text datablock.
  • The editor now has syntax highlighting (SH) and line numbers (LN) enabled, but greyed out and not active, because the data block is named 'Text'.
  • Renaming the data block to 'Text.py' activates SH+LN and enables the menu items & buttons.

I understand the niceities of opening a file and having SH+LN enabled or disabled based on the file extension. However, when trying to create new Python code in the text editor I think it's still a bit too much magic. This is aggravated by the text block being named 'Text' by default, making it less like a file name and more like a generic datablock name. We could improve this by changing the default datablock name to 'Text.txt' or 'Text.py', showing that this name actually behaves a bit like a file name and has a meaningful extension.

+1 to Brecht's comments, we'd need to update versioning_default.c too.

What should be done there that wasn't already done in rBc358da6b21158792bc05fa17fa00440675d0e76e ?

  • The editor now has syntax highlighting (SH) and line numbers (LN) enabled, but greyed out and not active, because the data block is named 'Text'.

What I suggested was to still do syntax highlighting in that case. But it's more of a workaround for the lack of extension.

I understand the niceities of opening a file and having SH+LN enabled or disabled based on the file extension. However, when trying to create new Python code in the text editor I think it's still a bit too much magic. This is aggravated by the text block being named 'Text' by default, making it less like a file name and more like a generic datablock name. We could improve this by changing the default datablock name to 'Text.txt' or 'Text.py', showing that this name actually behaves a bit like a file name and has a meaningful extension.

We could have separate "New Python Script" and "New Text" in the menu, and add .py and .txt extensions for those.

+1 to Brecht's comments, we'd need to update versioning_default.c too.

What should be done there that wasn't already done in rBc358da6b21158792bc05fa17fa00440675d0e76e ?

Nothing is needed then. Recall this was done for the Scripting Workspace, however it wasn't limited to that - so this patch is fine as-is.

Partial implementation of the smartness @Brecht Van Lommel (brecht) suggested.

The syntax highlighting (SH) and line numbers (LN) menu items +
buttons are now disabled when there is a non-highlighted,
non-numerical file extension. The thing I'm still a bit in doubt about
is how to handle the actual disabling of SH+LN without setting
st->showsyntax and st->showlinenrs to false. It seems a bit
inefficient to keep comparing the extension against the list of syntax
highlighters every time those properties are used.

One way of solving this could be changing the type of those fields
from bool to enum and have values like ON, OFF, and
TEMPORARY_OFF. The latter would mean "the user turned it on, but
it's turned off due to the file extension being non-highlighted".

Another way of solving it would be to just ignore the performance
effects and just do the check all the time. And there are of course
more ways to solve this, which is why I'd like some feedback first.

  • Removed unused #include
Campbell Barton (campbellbarton) requested changes to this revision.Wed, Aug 14, 4:17 PM

Comments inline.

release/scripts/startup/bl_ui/space_text.py
55 ↗(On Diff #17109)

It's odd that line numbers are disabled too, this isn't related to syntax highlighting (again below).

source/blender/blenlib/intern/path_util.c
1697 ↗(On Diff #17109)

Might as well check BLI_first_slash here.

source/blender/blenlib/intern/string_utils.c
92 ↗(On Diff #17109)

Prefer more verbose & readable loop.

while (isdigit(*string)) {
  string++;
}
source/blender/makesrna/intern/rna_space.c
4579 ↗(On Diff #17109)

Name is_highlightable is a bit odd.

We have show_syntax_highlight, check for highlight could be is_syntax_highlight_supported.

4579–4584 ↗(On Diff #17109)

Would make this a property of the text, not the text space.

This revision now requires changes to proceed.Wed, Aug 14, 4:17 PM
Sybren A. Stüvel (sybren) marked 4 inline comments as done.Wed, Aug 14, 4:25 PM
Sybren A. Stüvel (sybren) updated this revision to Diff 17110.
release/scripts/startup/bl_ui/space_text.py
55 ↗(On Diff #17109)

I'm fine just doing this for syntax highlighting only.

source/blender/makesrna/intern/rna_space.c
4579–4584 ↗(On Diff #17109)

This I did to keep the same scope as show_syntax_highlight, and to make it easier to simply use st.is_syntax_highlight_supported() (as opposed to st.text is not None and st.text.is_syntax_highlight_supported()).

  • Only apply is_syntax_highlight_supported() to syntax highlighting
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/include/ED_text.h
43

Convention in ED_ headers is just to use filename, as above with text_undo.c.

source/blender/makesrna/intern/rna_space.c
4579–4584 ↗(On Diff #17109)

If it we would ever want to call this on a text datablock it would be odd if it needed to be assigned a text space first.

Since it seems unlikely, I don't have a strong opinion on this, fine to keep here too.

This revision is now accepted and ready to land.Wed, Aug 14, 4:31 PM

Override the st->showsyntax setting when highlighting is not supported.

I've extended TextDrawContext with a syntax_highlight property, which is only set true if syntax highlighting is enabled by the user and actually supported for the file extension.

Sybren A. Stüvel (sybren) marked 2 inline comments as done.Wed, Aug 14, 4:45 PM
Sybren A. Stüvel (sybren) added inline comments.
source/blender/makesrna/intern/rna_space.c
4579–4584 ↗(On Diff #17109)

Trivial to add to the Text RNA as well, so I just did that too.

Sybren A. Stüvel (sybren) marked an inline comment as done.Wed, Aug 14, 4:45 PM
Sybren A. Stüvel (sybren) updated this revision to Diff 17114.
  • Final feedback tweaks
  • Added missing forward declaration

Tweaks to space_text.py to minimise changes