This patch fixes the problems with indentation via keyboard shortcut and with inconsistent indentation whitespaces after pasting from another source by converting all pasted indentation whitespaces into the character defined by the "use tabs as spaces" option.
Details
Details
- Reviewers
Campbell Barton (campbellbarton) Sybren A. Stüvel (sybren) Brecht Van Lommel (brecht) - Group Reviewers
Text Editor - Maniphest Tasks
- T60234: Shift+Tab only works with new text
- Commits
- rBSb4c14faeaf6f: Text editor: convert tabs to spaces on paste
rBb4c14faeaf6f: Text editor: convert tabs to spaces on paste
Diff Detail
Diff Detail
Event Timeline
| source/blender/editors/space_text/text_ops.c | ||
|---|---|---|
| 70 | This is not a reference but a pointer. I know C doesn't really have references, but since Blender also has a large body of code in C++ we ought to be careful in the terminology. | |
| 83 | Those while statements can be replaced by for (i=0; (*buf)[i]; i++). Every iteration i++ is executed anyway, and both while statements start with i=0. | |
| 85 | This if/else if construct is not very trivial and repeated twice. Maybe it's a good one to split it into a small function and call it three times? | |
| 107 | This replaces a \t with tab_size spaces, which is incorrect. It should align the text with the next tab stop. " \t" should generally result in the same indentation as "\t" (unless that one space would push it to the next tab stop, in which case it's equivalent to "\t\t". | |
| 821 | This boolean is only used once, so just change the condition below to if (text->flags & TXT_TABSTOSPACES == 0). | |
Comment Actions
This doesn't fix the main bug in T60234: Shift+Tab only works with new text, which I think is that our indent/unindent operators should work with both tabs and spaces. But we can consider this as a change on its own.
Converting tabs to spaces automatically on paste makes some sense if you interpret paste as similar to typing in the code. When using the text editor for something else than Python scripting it might be unexpected though, perhaps it's an acceptable trade-off.
| source/blender/editors/space_text/text_ops.c | ||
|---|---|---|
| 833–835 | I don't think it's right to convert all spaces to tabs when this option is off, probably it should do nothing then. When using tabs, you sometimes indent some part with spaces intentionally to ensure things align regardless of tab size. | |
| source/blender/editors/space_text/text_ops.c | ||
|---|---|---|
| 833–835 | The buf_spaces_to_tabs function shouldn't change indentation; so given a tab size of 4 it should convert 7 spaces to 1 tab + 3 spaces. Mixing tabs with spaces for indentation is invalid in Python, which brings us again to when to apply these transformations (always or only when editing Python code). | |
| source/blender/editors/space_text/text_ops.c | ||
|---|---|---|
| 833–835 | Given these two problems, when tabs_to_spaces == false, should I keep the pasted text original and let the user deal with it? | |
Comment Actions
off topic: Should I post the code as I mark the comments as done, or should I wait until every comment is solved to post?
Comment Actions
Generally I mark them as 'done' as I do them (as to give me a reminder of what's left to do) but not submit, and submit just before I post an update to the diff.
Comment Actions
I have implemented the suggestions in code style and structure.
As converting from tabs could lead to unexpected behavior, I kept the text unmodified in this case.
| source/blender/editors/space_text/text_ops.c | ||
|---|---|---|
| 812 | Since buf_tabs_to_spaces is only called with true as the last argument, do we really need that parameter to be there? | |
Comment Actions
That flag free_src was there in case of reuse of the function, but I think there will not be many reuses, so removing it may be better.
Comment Actions
Hi, just as a reminder of this DIff I am bumping this thread. Sorry if this is not allowed, but it has been a while since my last diff update.
Comment Actions
I'll commit this with some tweaks for code clarity.
Also fixing a missing reset of spaces_until_tab when starting a new line.