Changeset View
Standalone View
source/blender/editors/space_text/text_ops.c
| Show First 20 Lines • Show All 55 Lines • ▼ Show 20 Lines | |||||
| # include "BPY_extern.h" | # include "BPY_extern.h" | ||||
| #endif | #endif | ||||
| #include "text_intern.h" | #include "text_intern.h" | ||||
| #include "text_format.h" | #include "text_format.h" | ||||
| static void txt_screen_clamp(SpaceText *st, ARegion *ar); | static void txt_screen_clamp(SpaceText *st, ARegion *ar); | ||||
| /************************ util ***************************/ | |||||
| /** | |||||
| * Tests if the given character represents a start of a new line or the | |||||
| * indentation part of a line. | |||||
| * \param c: The current character. | |||||
sybren: This comment seems to be outdated now. There is no `line_start` flag. | |||||
| * \param r_last_state: A pointer to a flag representing the last state. The | |||||
Done Inline ActionsThis 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. sybren: This is not a reference but a pointer. I know C doesn't really have references, but since… | |||||
| * flag may be modified. | |||||
| */ | |||||
Done Inline Actionslast_state should be renamed to r_last_state, and here also line_start doesn't exist, so the comment doesn't make much sense. sybren: `last_state` should be renamed to `r_last_state`, and here also `line_start` doesn't exist, so… | |||||
| static void test_line_start(char c, bool *r_last_state) | |||||
| { | |||||
Done Inline ActionsRename last_state to r_last_state to indicate that it is used for returning a value. sybren: Rename `last_state` to `r_last_state` to indicate that it is used for returning a value. | |||||
| if (c == '\n') { | |||||
| *r_last_state = true; | |||||
| } | |||||
| else if (!ELEM(c, '\t', ' ')) { | |||||
| *r_last_state = false; | |||||
| } | |||||
| } | |||||
| /** | |||||
Done Inline ActionsThose 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. sybren: Those `while` statements can be replaced by `for (i=0; (*buf)[i]; i++)`. Every iteration `i++`… | |||||
| * This function converts the indentation tabs from a buffer to spaces. | |||||
| * \param buf: A pointer to a cstring. | |||||
Done Inline ActionsThis 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? sybren: This `if`/`else if` construct is not very trivial and repeated twice. Maybe it's a good one to… | |||||
| * \param tab_size: The size, in spaces, of the tab character. | |||||
| */ | |||||
| static char *buf_tabs_to_spaces(const char *in_buf, const int tab_size) | |||||
| { | |||||
| /* Get the number of tab characters in buffer. */ | |||||
| bool line_start = true; | |||||
| int num_tabs = 0; | |||||
| for (int in_offset = 0; in_buf[in_offset]; in_offset++) { | |||||
| /* Verify if is an indentation whitespace character. */ | |||||
| test_line_start(in_buf[in_offset], &line_start); | |||||
| if (in_buf[in_offset] == '\t' && line_start) { | |||||
| num_tabs++; | |||||
| } | |||||
| } | |||||
| /* Allocate output before with extra space for expanded tabs. */ | |||||
| const int out_size = strlen(in_buf) + num_tabs * (tab_size - 1); | |||||
| char *out_buf = MEM_mallocN(out_size * sizeof(char), __func__); | |||||
| /* Fill output buffer. */ | |||||
Done Inline ActionsThis 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". sybren: This replaces a `\t` with `tab_size` spaces, which is incorrect. It should align the text with… | |||||
| int spaces_until_tab = 0; | |||||
| int out_offset = 0; | |||||
| line_start = true; | |||||
| for (int in_offset = 0; in_buf[in_offset]; in_offset++) { | |||||
| /* Verify if is an indentation whitespace character. */ | |||||
| test_line_start(in_buf[in_offset], &line_start); | |||||
| if (in_buf[in_offset] == '\t' && line_start) { | |||||
| /* Calculate tab size so it fills until next indentation. */ | |||||
| int num_spaces = tab_size - (spaces_until_tab % tab_size); | |||||
| spaces_until_tab = 0; | |||||
| /* Write to buffer. */ | |||||
| memset(&out_buf[out_offset], ' ', num_spaces); | |||||
| out_offset += num_spaces; | |||||
| } | |||||
| else { | |||||
| if (in_buf[in_offset] == ' ') { | |||||
| spaces_until_tab++; | |||||
| } | |||||
| else if (in_buf[in_offset] == '\n') { | |||||
| spaces_until_tab = 0; | |||||
| } | |||||
Done Inline ActionsThe above if-block does not change line_start, so if (!line_start) should just be replaced with else. sybren: The above `if`-block does not change `line_start`, so `if (!line_start)` should just be… | |||||
| out_buf[out_offset++] = in_buf[in_offset]; | |||||
| } | |||||
| } | |||||
| out_buf[out_offset] = '\0'; | |||||
| return out_buf; | |||||
| } | |||||
| /************************ poll ***************************/ | /************************ poll ***************************/ | ||||
| BLI_INLINE int text_pixel_x_to_column(SpaceText *st, const int x) | BLI_INLINE int text_pixel_x_to_column(SpaceText *st, const int x) | ||||
| { | { | ||||
| /* add half the char width so mouse cursor selection is inbetween letters */ | /* add half the char width so mouse cursor selection is inbetween letters */ | ||||
| return (x + (st->cwidth / 2)) / st->cwidth; | return (x + (st->cwidth / 2)) / st->cwidth; | ||||
| } | } | ||||
| ▲ Show 20 Lines • Show All 655 Lines • ▼ Show 20 Lines | for (ob = CTX_data_main(C)->objects.first; ob; ob = ob->id.next) { | ||||
| if (update) { | if (update) { | ||||
| DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY); | DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY); | ||||
| } | } | ||||
| } | } | ||||
| # endif | # endif | ||||
| #endif | #endif | ||||
| return OPERATOR_FINISHED; | return OPERATOR_FINISHED; | ||||
| } | } | ||||
Done Inline ActionsSince buf_tabs_to_spaces is only called with true as the last argument, do we really need that parameter to be there? sybren: Since `buf_tabs_to_spaces` is only called with `true` as the last argument, do we really need… | |||||
| void TEXT_OT_refresh_pyconstraints(wmOperatorType *ot) | void TEXT_OT_refresh_pyconstraints(wmOperatorType *ot) | ||||
| { | { | ||||
| /* identifiers */ | /* identifiers */ | ||||
| ot->name = "Refresh PyConstraints"; | ot->name = "Refresh PyConstraints"; | ||||
| ot->idname = "TEXT_OT_refresh_pyconstraints"; | ot->idname = "TEXT_OT_refresh_pyconstraints"; | ||||
| ot->description = "Refresh all pyconstraints"; | ot->description = "Refresh all pyconstraints"; | ||||
| /* api callbacks */ | /* api callbacks */ | ||||
Done Inline ActionsThis boolean is only used once, so just change the condition below to if (text->flags & TXT_TABSTOSPACES == 0). sybren: This boolean is only used once, so just change the condition below to `if (text->flags &… | |||||
| ot->exec = text_refresh_pyconstraints_exec; | ot->exec = text_refresh_pyconstraints_exec; | ||||
| ot->poll = text_edit_poll; | ot->poll = text_edit_poll; | ||||
| } | } | ||||
| /******************* paste operator *********************/ | /******************* paste operator *********************/ | ||||
| static int text_paste_exec(bContext *C, wmOperator *op) | static int text_paste_exec(bContext *C, wmOperator *op) | ||||
| { | { | ||||
| const bool selection = RNA_boolean_get(op->ptr, "selection"); | const bool selection = RNA_boolean_get(op->ptr, "selection"); | ||||
| Text *text = CTX_data_edit_text(C); | Text *text = CTX_data_edit_text(C); | ||||
| char *buf; | char *buf; | ||||
| int buf_len; | int buf_len; | ||||
| buf = WM_clipboard_text_get(selection, &buf_len); | buf = WM_clipboard_text_get(selection, &buf_len); | ||||
Done Inline ActionsI 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. brecht: I don't think it's right to convert all spaces to tabs when this option is off, probably it… | |||||
Done Inline ActionsThe 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). sybren: The `buf_spaces_to_tabs` function shouldn't change indentation; so given a tab size of 4 it… | |||||
Done Inline ActionsGiven these two problems, when tabs_to_spaces == false, should I keep the pasted text original and let the user deal with it? brunobbs: Given these two problems, when `tabs_to_spaces == false`, should I keep the pasted text… | |||||
| if (!buf) { | if (!buf) { | ||||
| return OPERATOR_CANCELLED; | return OPERATOR_CANCELLED; | ||||
| } | } | ||||
| text_drawcache_tag_update(CTX_wm_space_text(C), 0); | text_drawcache_tag_update(CTX_wm_space_text(C), 0); | ||||
| TextUndoBuf *utxt = ED_text_undo_push_init(C); | TextUndoBuf *utxt = ED_text_undo_push_init(C); | ||||
| /* Convert clipboard content indentation to spaces if specified */ | |||||
| if (text->flags & TXT_TABSTOSPACES) { | |||||
| char *new_buf = buf_tabs_to_spaces(buf, TXT_TABSIZE); | |||||
| MEM_freeN(buf); | |||||
| buf = new_buf; | |||||
| } | |||||
| txt_insert_buf(text, utxt, buf); | txt_insert_buf(text, utxt, buf); | ||||
| text_update_edited(text); | text_update_edited(text); | ||||
| MEM_freeN(buf); | MEM_freeN(buf); | ||||
| text_update_cursor_moved(C); | text_update_cursor_moved(C); | ||||
| WM_event_add_notifier(C, NC_TEXT | NA_EDITED, text); | WM_event_add_notifier(C, NC_TEXT | NA_EDITED, text); | ||||
| ▲ Show 20 Lines • Show All 2,745 Lines • Show Last 20 Lines | |||||
This comment seems to be outdated now. There is no line_start flag.