From 06347a64cac5e33574713a59ace9d1d0ea4b6f82 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sat, 25 May 2024 05:19:46 +0800 Subject: vim-patch:9.1.0443: Can't use blockwise selection with width for getregion() (#28985) Problem: Can't use a blockwise selection with a width for getregion(). Solution: Add support for blockwise selection with width like the return value of getregtype() or the "regtype" value of TextYankPost (zeertzjq). closes: vim/vim#14842 https://github.com/vim/vim/commit/afc2295c2201ae87bfbb42d5f5315ad0583ccabf --- src/nvim/eval.lua | 11 +++++------ src/nvim/eval/funcs.c | 30 +++++++++++++++++++----------- 2 files changed, 24 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua index 7d4438ded6..4adf3ce959 100644 --- a/src/nvim/eval.lua +++ b/src/nvim/eval.lua @@ -4370,14 +4370,13 @@ M.funcs = { The optional argument {opts} is a Dict and supports the following items: - type Specify the region's selection type - (default: "v"): - "v" for |charwise| mode - "V" for |linewise| mode - "" for |blockwise-visual| mode + type Specify the region's selection type. + See |getregtype()| for possible values, + except it cannot be an empty string. + (default: "v") exclusive If |TRUE|, use exclusive selection - for the end position + for the end position. (default: follow 'selection') You can get the last selection type by |visualmode()|. diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index e4cb63eb8e..3cb6ef41c0 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -2824,7 +2824,7 @@ static char *block_def2str(struct block_def *bd) } static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2, - bool *const inclusive, MotionType *region_type, oparg_T *oa) + bool *const inclusive, MotionType *region_type, oparg_T *oap) FUNC_ATTR_NONNULL_ALL { tv_list_alloc_ret(rettv, kListLenMayKnow); @@ -2858,11 +2858,17 @@ static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2 type = default_type; } + int block_width = 0; if (type[0] == 'v' && type[1] == NUL) { *region_type = kMTCharWise; } else if (type[0] == 'V' && type[1] == NUL) { *region_type = kMTLineWise; - } else if (type[0] == Ctrl_V && type[1] == NUL) { + } else if (type[0] == Ctrl_V) { + char *p = type + 1; + if (*p != NUL && ((block_width = getdigits_int(&p, false, 0)) <= 0 || *p != NUL)) { + semsg(_(e_invargNval), "type", type); + return FAIL; + } *region_type = kMTBlockWise; } else { semsg(_(e_invargNval), "type", type); @@ -2926,16 +2932,18 @@ static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2 colnr_T sc1, ec1, sc2, ec2; getvvcol(curwin, p1, &sc1, NULL, &ec1); getvvcol(curwin, p2, &sc2, NULL, &ec2); - oa->motion_type = kMTBlockWise; - oa->inclusive = true; - oa->op_type = OP_NOP; - oa->start = *p1; - oa->end = *p2; - oa->start_vcol = MIN(sc1, sc2); - if (is_select_exclusive && ec1 < sc2 && 0 < sc2 && ec2 > ec1) { - oa->end_vcol = sc2 - 1; + oap->motion_type = kMTBlockWise; + oap->inclusive = true; + oap->op_type = OP_NOP; + oap->start = *p1; + oap->end = *p2; + oap->start_vcol = MIN(sc1, sc2); + if (block_width > 0) { + oap->end_vcol = oap->start_vcol + block_width - 1; + } else if (is_select_exclusive && ec1 < sc2 && 0 < sc2 && ec2 > ec1) { + oap->end_vcol = sc2 - 1; } else { - oa->end_vcol = MAX(ec1, ec2); + oap->end_vcol = MAX(ec1, ec2); } } -- cgit From 8b2b1fba2abfb99186e3a1f0123251a3e2eae3fe Mon Sep 17 00:00:00 2001 From: glepnir Date: Fri, 3 May 2024 15:53:13 +0800 Subject: fix(float): missing default highlight for title Problem: there is missing default title highlight when highlight not defined in title text chunk. Solution: when attr is not set use default title highlight group. --- src/nvim/api/win_config.c | 9 +++------ src/nvim/drawscreen.c | 12 ++++++++---- 2 files changed, 11 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 3a9986a7d1..70235d8db6 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -189,13 +189,13 @@ /// ``` /// - title: Title (optional) in window border, string or list. /// List should consist of `[text, highlight]` tuples. -/// If string, the default highlight group is `FloatTitle`. +/// If string, or a tuple lacks a highlight, the default highlight group is `FloatTitle`. /// - title_pos: Title position. Must be set with `title` option. /// Value can be one of "left", "center", or "right". /// Default is `"left"`. /// - footer: Footer (optional) in window border, string or list. /// List should consist of `[text, highlight]` tuples. -/// If string, the default highlight group is `FloatFooter`. +/// If string, or a tuple lacks a highlight, the default highlight group is `FloatFooter`. /// - footer_pos: Footer position. Must be set with `footer` option. /// Value can be one of "left", "center", or "right". /// Default is `"left"`. @@ -851,7 +851,6 @@ static void parse_bordertext(Object bordertext, BorderTextType bordertext_type, bool *is_present; VirtText *chunks; int *width; - int default_hl_id; switch (bordertext_type) { case kBorderTextTitle: if (fconfig->title) { @@ -861,7 +860,6 @@ static void parse_bordertext(Object bordertext, BorderTextType bordertext_type, is_present = &fconfig->title; chunks = &fconfig->title_chunks; width = &fconfig->title_width; - default_hl_id = syn_check_group(S_LEN("FloatTitle")); break; case kBorderTextFooter: if (fconfig->footer) { @@ -871,7 +869,6 @@ static void parse_bordertext(Object bordertext, BorderTextType bordertext_type, is_present = &fconfig->footer; chunks = &fconfig->footer_chunks; width = &fconfig->footer_width; - default_hl_id = syn_check_group(S_LEN("FloatFooter")); break; } @@ -881,7 +878,7 @@ static void parse_bordertext(Object bordertext, BorderTextType bordertext_type, return; } kv_push(*chunks, ((VirtTextChunk){ .text = xstrdup(bordertext.data.string.data), - .hl_id = default_hl_id })); + .hl_id = -1 })); *width = (int)mb_string2cells(bordertext.data.string.data); *is_present = true; return; diff --git a/src/nvim/drawscreen.c b/src/nvim/drawscreen.c index bda0ccc870..039bbd219c 100644 --- a/src/nvim/drawscreen.c +++ b/src/nvim/drawscreen.c @@ -65,6 +65,7 @@ #include "nvim/autocmd.h" #include "nvim/autocmd_defs.h" #include "nvim/buffer.h" +#include "nvim/buffer_defs.h" #include "nvim/charset.h" #include "nvim/cmdexpand.h" #include "nvim/cursor.h" @@ -715,14 +716,17 @@ void end_search_hl(void) screen_search_hl.rm.regprog = NULL; } -static void win_redr_bordertext(win_T *wp, VirtText vt, int col) +static void win_redr_bordertext(win_T *wp, VirtText vt, int col, BorderTextType bt) { for (size_t i = 0; i < kv_size(vt);) { - int attr = 0; + int attr = -1; char *text = next_virt_text_chunk(vt, &i, &attr); if (text == NULL) { break; } + if (attr == -1) { // No highlight specified. + attr = wp->w_ns_hl_attr[bt == kBorderTextTitle ? HLF_BTITLE : HLF_BFOOTER]; + } attr = hl_apply_winblend(wp, attr); col += grid_line_puts(col, text, -1, attr); } @@ -773,7 +777,7 @@ static void win_redr_border(win_T *wp) if (wp->w_config.title) { int title_col = win_get_bordertext_col(icol, wp->w_config.title_width, wp->w_config.title_pos); - win_redr_bordertext(wp, wp->w_config.title_chunks, title_col); + win_redr_bordertext(wp, wp->w_config.title_chunks, title_col, kBorderTextTitle); } if (adj[1]) { grid_line_put_schar(icol + adj[3], chars[2], attrs[2]); @@ -809,7 +813,7 @@ static void win_redr_border(win_T *wp) if (wp->w_config.footer) { int footer_col = win_get_bordertext_col(icol, wp->w_config.footer_width, wp->w_config.footer_pos); - win_redr_bordertext(wp, wp->w_config.footer_chunks, footer_col); + win_redr_bordertext(wp, wp->w_config.footer_chunks, footer_col, kBorderTextFooter); } if (adj[1]) { grid_line_put_schar(icol + adj[3], chars[4], attrs[4]); -- cgit From 4757d497f3c85cc343f7dcbc09f95e43ba5c1314 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sat, 25 May 2024 16:53:10 +0800 Subject: vim-patch:9.1.0444: Not enough tests for getregion() with multibyte chars (#29000) Problem: Not enough tests for getregion() with multibyte chars. Solution: Add a few more tests (zeertzjq). closes: vim/vim#14844 https://github.com/vim/vim/commit/dff55a335889c746a79974f7c52cdcdebad682c2 --- src/nvim/eval.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua index 4adf3ce959..f0c8963459 100644 --- a/src/nvim/eval.lua +++ b/src/nvim/eval.lua @@ -4372,7 +4372,8 @@ M.funcs = { type Specify the region's selection type. See |getregtype()| for possible values, - except it cannot be an empty string. + except that the width can be omitted + and an empty string cannot be used. (default: "v") exclusive If |TRUE|, use exclusive selection -- cgit From fdeb01cd77404aba446c67af32134a2ff793a14b Mon Sep 17 00:00:00 2001 From: Rafael Kitover Date: Sat, 25 May 2024 12:44:39 +0000 Subject: feat(main): expand file ~\ or ~/ prefix on Windows (#28515) In command_line_scan() for MSWIN, expand "~\" or "~/" prefixed paths to the USERPROFILE environment variable for the user's profile directory. Fix #23901 Signed-off-by: Rafael Kitover --- src/nvim/main.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src') diff --git a/src/nvim/main.c b/src/nvim/main.c index 30b6b6e86b..17a0bbf082 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -1444,6 +1444,19 @@ scripterror: ga_grow(&global_alist.al_ga, 1); char *p = xstrdup(argv[0]); + // On Windows expand "~\" or "~/" prefix in file names to profile directory. +#ifdef MSWIN + if (*p == '~' && (p[1] == '\\' || p[1] == '/')) { + char *profile_dir = vim_getenv("HOME"); + size_t size = strlen(profile_dir) + strlen(p); + char *tilde_expanded = xmalloc(size); + snprintf(tilde_expanded, size, "%s%s", profile_dir, p + 1); + xfree(p); + xfree(profile_dir); + p = tilde_expanded; + } +#endif + if (parmp->diff_mode && os_isdir(p) && GARGCOUNT > 0 && !os_isdir(alist_name(&GARGLIST[0]))) { char *r = concat_fnames(p, path_tail(alist_name(&GARGLIST[0])), true); -- cgit From bc63ffcf39e8ad6c0925c0ad8503bfb3ed8497f3 Mon Sep 17 00:00:00 2001 From: luukvbaal Date: Sun, 26 May 2024 19:54:08 +0200 Subject: fix(tui): reset clear_region attributes during startup #28713 Problem: Fix added in #28676 worked accidentally(used variables were themselves uninitialized at this point during startup) and does not always work. Solution: Reset attributes when clearing regions during startup. --- src/nvim/tui/tui.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c index 2a9530defb..dc8c8def5b 100644 --- a/src/nvim/tui/tui.c +++ b/src/nvim/tui/tui.c @@ -115,6 +115,7 @@ struct TUIData { kvec_t(HlAttrs) attrs; int print_attr_id; bool default_attr; + bool set_default_colors; bool can_clear_attr; ModeShape showing_mode; Integer verbose; @@ -166,14 +167,6 @@ void tui_start(TUIData **tui_p, int *width, int *height, char **term, bool *rgb) tui->seen_error_exit = 0; tui->loop = &main_loop; tui->url = -1; - // Because setting the default colors is delayed until after startup to avoid - // flickering with the default colorscheme background, any flush that happens - // during startup in turn would result in clearing invalidated regions with - // uninitialized attrs(black). Instead initialize clear_attrs with current - // terminal background so that it is at least not perceived as flickering, even - // though it may be different from the colorscheme that is set during startup. - tui->clear_attrs.rgb_bg_color = normal_bg; - tui->clear_attrs.cterm_bg_color = (int16_t)cterm_normal_bg_color; kv_init(tui->invalid_regions); kv_init(tui->urlbuf); @@ -1016,7 +1009,16 @@ static void clear_region(TUIData *tui, int top, int bot, int left, int right, in { UGrid *grid = &tui->grid; - update_attrs(tui, attr_id); + // Setting the default colors is delayed until after startup to avoid flickering + // with the default colorscheme background. Consequently, any flush that happens + // during startup would result in clearing invalidated regions with zeroed + // clear_attrs, perceived as a black flicker. Reset attributes to clear with + // current terminal background instead(#28667, #28668). + if (tui->set_default_colors) { + update_attrs(tui, attr_id); + } else { + unibi_out(tui, unibi_exit_attribute_mode); + } // Background is set to the default color and the right edge matches the // screen end, try to use terminal codes for clearing the requested area. @@ -1419,6 +1421,7 @@ void tui_default_colors_set(TUIData *tui, Integer rgb_fg, Integer rgb_bg, Intege tui->clear_attrs.cterm_bg_color = (int16_t)cterm_bg; tui->print_attr_id = -1; + tui->set_default_colors = true; invalidate(tui, 0, tui->grid.height, 0, tui->grid.width); } -- cgit From 43a2019f09e855c4eae33bfdbdec4cc7b2985075 Mon Sep 17 00:00:00 2001 From: luukvbaal Date: Sun, 26 May 2024 19:55:57 +0200 Subject: fix(extmarks): issues with revalidating marks #28961 Problem: Invalid marks appear to be revalidated multiple times, and decor is added at the old position for unpaired marks. Solution: Avoid revalidating already valid marks, and don't use old position to add to decor for unpaired marks. --- src/nvim/extmark.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/nvim/extmark.c b/src/nvim/extmark.c index 3236590010..4e47fa76fe 100644 --- a/src/nvim/extmark.c +++ b/src/nvim/extmark.c @@ -117,9 +117,16 @@ static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool MarkTreeIter itr[1] = { 0 }; MTKey key = marktree_lookup(buf->b_marktree, mark, itr); if (key.pos.row < 0 || (key.pos.row == row && key.pos.col == col)) { + // Does this hold? If it doesn't, we should still revalidate. + assert(!invalid || !mt_invalid(key)); return; } + // Key already revalidated(how?) Avoid adding to decor again. + if (invalid && !mt_invalid(key)) { + invalid = false; + } + // Only the position before undo needs to be redrawn here, // as the position after undo should be marked as changed. if (!invalid && mt_decor_any(key) && key.pos.row != row) { @@ -140,8 +147,8 @@ static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool marktree_move(buf->b_marktree, itr, row, col); if (invalid) { - MTPos end = marktree_get_altpos(buf->b_marktree, key, NULL); - buf_put_decor(buf, mt_decor(key), row, end.row); + row2 = mt_paired(key) ? marktree_get_altpos(buf->b_marktree, key, NULL).row : row; + buf_put_decor(buf, mt_decor(key), row, row2); } else if (key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) { buf_signcols_count_range(buf, row1, row2, 0, kNone); } -- cgit From 22fe04452e12944b409c8ee92cf4069ac8026987 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 27 May 2024 05:50:49 +0800 Subject: vim-patch:9.1.0446: getregionpos() inconsistent for partly-selected multibyte char (#29032) Problem: getregionpos() behaves inconsistently for a partly-selected multibyte char. Solution: Always use column of the first byte for a partly-selected multibyte char (zeertzjq). closes: vim/vim#14851 https://github.com/vim/vim/commit/ef73374dc3e4bf8104ba31d5b22517f8028b467a --- src/nvim/eval/funcs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 3cb6ef41c0..8b22c7a797 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -3042,6 +3042,7 @@ static void f_getregionpos(typval_T *argvars, typval_T *rettv, EvalFuncData fptr for (linenr_T lnum = p1.lnum; lnum <= p2.lnum; lnum++) { pos_T ret_p1, ret_p2; + char *line = ml_get(lnum); colnr_T line_len = ml_get_len(lnum); if (region_type == kMTLineWise) { @@ -3060,7 +3061,7 @@ static void f_getregionpos(typval_T *argvars, typval_T *rettv, EvalFuncData fptr if (bd.is_oneChar) { // selection entirely inside one char if (region_type == kMTBlockWise) { - ret_p1.col = bd.textcol; + ret_p1.col = (colnr_T)(mb_prevptr(line, bd.textstart) - line) + 1; ret_p1.coladd = bd.start_char_vcols - (bd.start_vcol - oa.start_vcol); } else { ret_p1.col = p1.col + 1; @@ -3072,7 +3073,7 @@ static void f_getregionpos(typval_T *argvars, typval_T *rettv, EvalFuncData fptr ret_p1.coladd = oa.start_vcol - bd.start_vcol; bd.is_oneChar = true; } else if (bd.startspaces > 0) { - ret_p1.col = bd.textcol; + ret_p1.col = (colnr_T)(mb_prevptr(line, bd.textstart) - line) + 1; ret_p1.coladd = bd.start_char_vcols - bd.startspaces; } else { ret_p1.col = bd.textcol + 1; -- cgit From 9a0239fdc8b380a8a32739a7c722fe90e3c2e910 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 27 May 2024 14:37:22 +0800 Subject: fix(drawline): don't draw beyond end of window (#29035) --- src/nvim/drawline.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/nvim/drawline.c b/src/nvim/drawline.c index 07944081da..4d534d78a2 100644 --- a/src/nvim/drawline.c +++ b/src/nvim/drawline.c @@ -1596,6 +1596,10 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, int col_rows, s // hide virt_text on text hidden by 'nowrap' or 'smoothscroll' decor_redraw_col(wp, (colnr_T)(ptr - line) - 1, wlv.off, true, &decor_state); } + if (wlv.col >= grid->cols) { + wlv.col = wlv.off = grid->cols; + goto end_check; + } } if (cul_screenline && wlv.filler_todo <= 0 @@ -2650,13 +2654,6 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, int col_rows, s conceal_cursor_used = conceal_cursor_line(curwin); } - // When the window is too narrow draw all "@" lines. - if (leftcols_width >= wp->w_grid.cols && is_wrapped) { - win_draw_end(wp, schar_from_ascii('@'), true, wlv.row, wp->w_grid.rows, HLF_AT); - set_empty_rows(wp, wlv.row); - wlv.row = endrow; - } - break; } @@ -2844,10 +2841,12 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, int col_rows, s } } +end_check: // At end of screen line and there is more to come: Display the line // so far. If there is no more to display it is caught above. if (wlv.col >= grid->cols && (!has_foldtext || virt_line_offset >= 0) - && (*ptr != NUL + && (wlv.col <= leftcols_width + || *ptr != NUL || wlv.filler_todo > 0 || (wp->w_p_list && wp->w_p_lcs_chars.eol != NUL && lcs_eol_todo) || (wlv.n_extra != 0 && (wlv.sc_extra != NUL || *wlv.p_extra != NUL)) -- cgit From 104800ce2eadd21475b5a4897265a8a177e58d77 Mon Sep 17 00:00:00 2001 From: Malte Dehling Date: Mon, 27 May 2024 04:14:57 -0700 Subject: build: "popcount" name conflict on NetBSD #28983 Problem: NetBSD's libc already has a function by the same name. Solution: Rename popcount to xpopcount and add #if defined(__NetBSD__) to prefer NetBSD's own implementation. This fixes #28983. --- src/nvim/api/vim.c | 4 ++-- src/nvim/math.c | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index fc780e1248..52ab18cbff 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -2354,8 +2354,8 @@ void nvim__redraw(Dict(redraw) *opts, Error *err) } } - int count = (win != NULL) + (buf != NULL); - VALIDATE(popcount(opts->is_set__redraw_) > count, "%s", "at least one action required", { + unsigned count = (win != NULL) + (buf != NULL); + VALIDATE(xpopcount(opts->is_set__redraw_) > count, "%s", "at least one action required", { return; }); diff --git a/src/nvim/math.c b/src/nvim/math.c index 1ccf4d7806..4ca212413b 100644 --- a/src/nvim/math.c +++ b/src/nvim/math.c @@ -78,13 +78,15 @@ int xctz(uint64_t x) } /// Count number of set bits in bit field. -int popcount(uint64_t x) +unsigned xpopcount(uint64_t x) { // Use compiler builtin if possible. -#if defined(__clang__) || defined(__GNUC__) - return __builtin_popcountll(x); +#if defined(__NetBSD__) + return popcount64(x); +#elif defined(__clang__) || defined(__GNUC__) + return (unsigned)__builtin_popcountll(x); #else - int count = 0; + unsigned count = 0; for (; x != 0; x >>= 1) { if (x & 1) { count++; -- cgit From dbc2678f46970703616765dadff1234fdcce27b7 Mon Sep 17 00:00:00 2001 From: glepnir Date: Mon, 27 May 2024 22:02:24 +0800 Subject: vim-patch:9.1.0447: completion may be wrong when deleting all chars (#29040) Problem: completion may be wrong when deleting all chars. Solution: reset compl_shown_match (glepnir). closes: https://github.com/vim/vim/pull/14854 https://github.com/vim/vim/commit/53387c55a13bc1013a6ab721d4bd0bd04c6935c4 --- src/nvim/insexpand.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/nvim/insexpand.c b/src/nvim/insexpand.c index b557b9802e..8b1c09b32f 100644 --- a/src/nvim/insexpand.c +++ b/src/nvim/insexpand.c @@ -1198,6 +1198,10 @@ static int ins_compl_build_pum(void) // match after it, don't highlight anything. bool shown_match_ok = match_at_original_text(compl_shown_match); + if (strequal(compl_leader, compl_orig_text) && !shown_match_ok) { + compl_shown_match = compl_no_select ? compl_first_match : compl_first_match->cp_next; + } + compl_T *shown_compl = NULL; bool did_find_shown_match = false; int cur = -1; -- cgit From 6e8a728e3dad747d0c46dc47a530b76e8997bc08 Mon Sep 17 00:00:00 2001 From: dundargoc Date: Sat, 25 May 2024 20:35:37 +0200 Subject: refactor: fix luals type warnings --- src/nvim/eval.lua | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua index f0c8963459..ceaba11f41 100644 --- a/src/nvim/eval.lua +++ b/src/nvim/eval.lua @@ -2133,6 +2133,7 @@ M.funcs = { name = 'exepath', params = { { 'expr', 'any' } }, signature = 'exepath({expr})', + returns = 'string', }, exists = { args = 1, -- cgit From 5b6477be45c54ebac4dce6bda51028542167fd1f Mon Sep 17 00:00:00 2001 From: luukvbaal Date: Tue, 28 May 2024 11:43:56 +0200 Subject: fix(ui): flush ext_cmdline events before doing cmdpreview #27950 Problem: Unable to update the screen for external cmdline during cmdpreview. Solution: Flush the cmdline UI before cmdpreview state. --- src/nvim/ex_getln.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c index f18dc0f747..cc2608433d 100644 --- a/src/nvim/ex_getln.c +++ b/src/nvim/ex_getln.c @@ -2532,6 +2532,10 @@ static bool cmdpreview_may_show(CommandLineState *s) goto end; } + // Flush now: external cmdline may itself wish to update the screen which is + // currently disallowed during cmdpreview(no longer needed in case that changes). + cmdline_ui_flush(); + // Swap invalid command range if needed if ((ea.argt & EX_RANGE) && ea.line1 > ea.line2) { linenr_T lnum = ea.line1; -- cgit From ff7f22c28b602c84350785624b4b6fc9ae35f950 Mon Sep 17 00:00:00 2001 From: bfredl Date: Tue, 28 May 2024 13:03:00 +0200 Subject: refactor(fileio): remove useless use of FileDescriptor FileDescriptor is used to buffer togheter many small writes to fewer syscalls. if the data to write already is in a single buffer, it is perfectly fine to just use os_write directly (which will take care of the reverse problem: splitting a too big write into many syscalls) --- src/nvim/main.c | 14 ++------------ src/nvim/os/fileio.c | 11 ----------- 2 files changed, 2 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/nvim/main.c b/src/nvim/main.c index 17a0bbf082..cf1324d37f 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -1100,23 +1100,13 @@ static void command_line_scan(mparm_T *parmp) // set stdout to binary to avoid crlf in --api-info output _setmode(STDOUT_FILENO, _O_BINARY); #endif - FileDescriptor fp; - const int fof_ret = file_open_fd(&fp, STDOUT_FILENO, - kFileWriteOnly); - if (fof_ret != 0) { - semsg(_("E5421: Failed to open stdin: %s"), os_strerror(fof_ret)); - } String data = api_metadata_raw(); - const ptrdiff_t written_bytes = file_write(&fp, data.data, data.size); + const ptrdiff_t written_bytes = os_write(STDOUT_FILENO, data.data, data.size, false); if (written_bytes < 0) { - msgpack_file_write_error((int)written_bytes); + semsg(_("E5420: Failed to write to file: %s"), os_strerror((int)written_bytes)); } - const int ff_ret = file_flush(&fp); - if (ff_ret < 0) { - msgpack_file_write_error(ff_ret); - } os_exit(0); } else if (STRICMP(argv[0] + argv_idx, "headless") == 0) { headless_mode = true; diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index da6fb13768..fbb2be5104 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -365,14 +365,3 @@ ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size) return (ptrdiff_t)read_bytes; } - -/// Print error which occurs when failing to write msgpack data -/// -/// @param[in] error Error code of the error to print. -/// -/// @return -1 (error return for msgpack_packer callbacks). -int msgpack_file_write_error(const int error) -{ - semsg(_("E5420: Failed to write to file: %s"), os_strerror(error)); - return -1; -} -- cgit From b386334cdbbc3e9d79773243fdbd53091488e14d Mon Sep 17 00:00:00 2001 From: bfredl Date: Sun, 26 May 2024 11:36:18 +0200 Subject: refactor(shada): remove ShaDaReadDef secondary wrapper `FileDescriptor` is already a wrapper around an fd and a buffer. By allowing to just use the buffer without an fd, it can already handle in-memory reads. --- src/nvim/os/fileio.c | 20 +++ src/nvim/os/fileio_defs.h | 2 + src/nvim/rbuffer.c | 19 ++- src/nvim/shada.c | 310 +++++++++++----------------------------------- 4 files changed, 113 insertions(+), 238 deletions(-) (limited to 'src') diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index da6fb13768..4575c394b8 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -126,6 +126,7 @@ int file_open_fd(FileDescriptor *const ret_fp, const int fd, const int flags) ret_fp->rv->data = ret_fp; ret_fp->rv->full_cb = (rbuffer_callback)&file_rb_write_full_cb; } + ret_fp->bytes_read = 0; return 0; } @@ -140,6 +141,18 @@ int file_open_stdin(FileDescriptor *fp) return error; } +/// opens buffer for reading +void file_open_buffer(FileDescriptor *ret_fp, char *data, size_t len) +{ + ret_fp->wr = false; + ret_fp->non_blocking = false; + ret_fp->fd = -1; + ret_fp->eof = true; + ret_fp->rv = rbuffer_new_wrap_buf(data, len); + ret_fp->_error = 0; + ret_fp->bytes_read = 0; +} + /// Close file and free its buffer /// /// @param[in,out] fp File to close. @@ -149,6 +162,11 @@ int file_open_stdin(FileDescriptor *fp) int file_close(FileDescriptor *const fp, const bool do_fsync) FUNC_ATTR_NONNULL_ALL { + if (fp->fd < 0) { + rbuffer_free(fp->rv); + return 0; + } + const int flush_error = (do_fsync ? file_fsync(fp) : file_flush(fp)); const int close_error = os_close(fp->fd); rbuffer_free(fp->rv); @@ -294,6 +312,7 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, const size_t fp->non_blocking); if (r_ret >= 0) { read_remaining -= (size_t)r_ret; + fp->bytes_read += (size - read_remaining); return (ptrdiff_t)(size - read_remaining); } else if (r_ret < 0) { return r_ret; @@ -314,6 +333,7 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, const size_t called_read = true; } } + fp->bytes_read += (size - read_remaining); return (ptrdiff_t)(size - read_remaining); } diff --git a/src/nvim/os/fileio_defs.h b/src/nvim/os/fileio_defs.h index 3dc8c7b22a..10277d2a7a 100644 --- a/src/nvim/os/fileio_defs.h +++ b/src/nvim/os/fileio_defs.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "nvim/func_attr.h" #include "nvim/rbuffer_defs.h" @@ -13,6 +14,7 @@ typedef struct { bool wr; ///< True if file is in write mode. bool eof; ///< True if end of file was encountered. bool non_blocking; ///< True if EAGAIN should not restart syscalls. + uint64_t bytes_read; ///< total bytes read so far } FileDescriptor; static inline bool file_eof(const FileDescriptor *fp) diff --git a/src/nvim/rbuffer.c b/src/nvim/rbuffer.c index 493c079d4c..cf2e10f90d 100644 --- a/src/nvim/rbuffer.c +++ b/src/nvim/rbuffer.c @@ -29,6 +29,23 @@ RBuffer *rbuffer_new(size_t capacity) return rv; } +/// Creates a new `RBuffer` instance for reading from a buffer. +/// +/// Must not be used with any write function like rbuffer_write_ptr or rbuffer_produced! +RBuffer *rbuffer_new_wrap_buf(char *data, size_t len) + FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_RET +{ + RBuffer *rv = xcalloc(1, sizeof(RBuffer)); + rv->full_cb = rv->nonfull_cb = NULL; + rv->data = NULL; + rv->size = len; + rv->read_ptr = data; + rv->write_ptr = data + len; + rv->end_ptr = NULL; + rv->temp = NULL; + return rv; +} + void rbuffer_free(RBuffer *buf) FUNC_ATTR_NONNULL_ALL { xfree(buf->temp); @@ -129,7 +146,7 @@ void rbuffer_consumed(RBuffer *buf, size_t count) assert(count <= buf->size); buf->read_ptr += count; - if (buf->read_ptr >= buf->end_ptr) { + if (buf->end_ptr && buf->read_ptr >= buf->end_ptr) { buf->read_ptr -= rbuffer_capacity(buf); } diff --git a/src/nvim/shada.c b/src/nvim/shada.c index 6f2eefeb92..597c7551fd 100644 --- a/src/nvim/shada.c +++ b/src/nvim/shada.c @@ -356,35 +356,6 @@ typedef struct { PMap(cstr_t) file_marks; ///< All file marks. } WriteMergerState; -typedef struct sd_read_def ShaDaReadDef; - -/// Function used to close files defined by ShaDaReadDef -typedef void (*ShaDaReadCloser)(ShaDaReadDef *const sd_reader) - REAL_FATTR_NONNULL_ALL; - -/// Function used to read ShaDa files -typedef ptrdiff_t (*ShaDaFileReader)(ShaDaReadDef *const sd_reader, - void *const dest, - const size_t size) - REAL_FATTR_NONNULL_ALL REAL_FATTR_WARN_UNUSED_RESULT; - -/// Function used to skip in ShaDa files -typedef int (*ShaDaFileSkipper)(ShaDaReadDef *const sd_reader, - const size_t offset) - REAL_FATTR_NONNULL_ALL REAL_FATTR_WARN_UNUSED_RESULT; - -/// Structure containing necessary pointers for reading ShaDa files -struct sd_read_def { - ShaDaFileReader read; ///< Reader function. - ShaDaReadCloser close; ///< Close function. - ShaDaFileSkipper skip; ///< Function used to skip some bytes. - void *cookie; ///< Data describing object read from. - bool eof; ///< True if reader reached end of file. - const char *error; ///< Error message in case of error. - uintmax_t fpos; ///< Current position (amount of bytes read since - ///< reader structure initialization). May overflow. -}; - #ifdef INCLUDE_GENERATED_DECLARATIONS # include "shada.c.generated.h" #endif @@ -587,70 +558,6 @@ static inline void hmll_dealloc(HMLList *const hmll) xfree(hmll->entries); } -/// Wrapper for reading from file descriptors -/// -/// @return -1 or number of bytes read. -static ptrdiff_t read_file(ShaDaReadDef *const sd_reader, void *const dest, const size_t size) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT -{ - const ptrdiff_t ret = file_read(sd_reader->cookie, dest, size); - sd_reader->eof = file_eof(sd_reader->cookie); - if (ret < 0) { - sd_reader->error = os_strerror((int)ret); - return -1; - } - sd_reader->fpos += (size_t)ret; - return ret; -} - -/// Read one character -static int read_char(ShaDaReadDef *const sd_reader) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT -{ - uint8_t ret; - ptrdiff_t read_bytes = sd_reader->read(sd_reader, &ret, 1); - if (read_bytes != 1) { - return EOF; - } - return (int)ret; -} - -/// Wrapper for closing file descriptors opened for reading -static void close_sd_reader(ShaDaReadDef *const sd_reader) - FUNC_ATTR_NONNULL_ALL -{ - close_file(sd_reader->cookie); - xfree(sd_reader->cookie); -} - -/// Wrapper for read that reads to IObuff and ignores bytes read -/// -/// Used for skipping. -/// -/// @param[in,out] sd_reader File read. -/// @param[in] offset Amount of bytes to skip. -/// -/// @return FAIL in case of failure, OK in case of success. May set -/// sd_reader->eof or sd_reader->error. -static int sd_reader_skip_read(ShaDaReadDef *const sd_reader, const size_t offset) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT -{ - const ptrdiff_t skip_bytes = file_skip(sd_reader->cookie, offset); - if (skip_bytes < 0) { - sd_reader->error = os_strerror((int)skip_bytes); - return FAIL; - } else if (skip_bytes != (ptrdiff_t)offset) { - assert(skip_bytes < (ptrdiff_t)offset); - sd_reader->eof = file_eof(sd_reader->cookie); - if (!sd_reader->eof) { - sd_reader->error = _("too few bytes read"); - } - return FAIL; - } - sd_reader->fpos += (size_t)skip_bytes; - return OK; -} - /// Wrapper for read that can be used when lseek cannot be used /// /// E.g. when trying to read from a pipe. @@ -660,55 +567,30 @@ static int sd_reader_skip_read(ShaDaReadDef *const sd_reader, const size_t offse /// /// @return kSDReadStatusReadError, kSDReadStatusNotShaDa or /// kSDReadStatusSuccess. -static ShaDaReadResult sd_reader_skip(ShaDaReadDef *const sd_reader, const size_t offset) +static ShaDaReadResult sd_reader_skip(FileDescriptor *const sd_reader, const size_t offset) FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL { - if (sd_reader->skip(sd_reader, offset) != OK) { - if (sd_reader->error != NULL) { - semsg(_(SERR "System error while skipping in ShaDa file: %s"), - sd_reader->error); - return kSDReadStatusReadError; - } else if (sd_reader->eof) { + const ptrdiff_t skip_bytes = file_skip(sd_reader, offset); + if (skip_bytes < 0) { + semsg(_(SERR "System error while skipping in ShaDa file: %s"), + os_strerror((int)skip_bytes)); + return kSDReadStatusReadError; + } else if (skip_bytes != (ptrdiff_t)offset) { + assert(skip_bytes < (ptrdiff_t)offset); + if (file_eof(sd_reader)) { semsg(_(RCERR "Error while reading ShaDa file: " "last entry specified that it occupies %" PRIu64 " bytes, " "but file ended earlier"), (uint64_t)offset); - return kSDReadStatusNotShaDa; + } else { + semsg(_(SERR "System error while skipping in ShaDa file: %s"), + _("too few bytes read")); } - abort(); + return kSDReadStatusNotShaDa; } return kSDReadStatusSuccess; } -/// Open ShaDa file for reading -/// -/// @param[in] fname File name to open. -/// @param[out] sd_reader Location where reader structure will be saved. -/// -/// @return libuv error in case of error, 0 otherwise. -static int open_shada_file_for_reading(const char *const fname, ShaDaReadDef *sd_reader) - FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL -{ - *sd_reader = (ShaDaReadDef) { - .read = &read_file, - .close = &close_sd_reader, - .skip = &sd_reader_skip_read, - .error = NULL, - .eof = false, - .fpos = 0, - .cookie = xmalloc(sizeof(FileDescriptor)), - }; - int error = file_open(sd_reader->cookie, fname, kFileReadOnly, 0); - if (error) { - XFREE_CLEAR(sd_reader->cookie); - return error; - } - - assert(strcmp(p_enc, "utf-8") == 0); - - return 0; -} - /// Wrapper for closing file descriptors static void close_file(FileDescriptor *cookie) { @@ -757,8 +639,8 @@ static int shada_read_file(const char *const file, const int flags) char *const fname = shada_filename(file); - ShaDaReadDef sd_reader; - const int of_ret = open_shada_file_for_reading(fname, &sd_reader); + FileDescriptor sd_reader; + int of_ret = file_open(&sd_reader, fname, kFileReadOnly, 0); if (p_verbose > 1) { verbose_enter(); @@ -782,7 +664,7 @@ static int shada_read_file(const char *const file, const int flags) xfree(fname); shada_read(&sd_reader, flags); - sd_reader.close(&sd_reader); + close_file(&sd_reader); return OK; } @@ -1094,7 +976,7 @@ static inline bool marks_equal(const pos_T a, const pos_T b) /// /// @param[in] sd_reader Structure containing file reader definition. /// @param[in] flags What to read, see ShaDaReadFileFlags enum. -static void shada_read(ShaDaReadDef *const sd_reader, const int flags) +static void shada_read(FileDescriptor *const sd_reader, const int flags) FUNC_ATTR_NONNULL_ALL { list_T *oldfiles_list = get_vim_var_list(VV_OLDFILES); @@ -1849,13 +1731,13 @@ static int compare_file_marks(const void *a, const void *b) /// /// @return kSDReadStatusNotShaDa, kSDReadStatusReadError or /// kSDReadStatusSuccess. -static inline ShaDaReadResult shada_parse_msgpack(ShaDaReadDef *const sd_reader, +static inline ShaDaReadResult shada_parse_msgpack(FileDescriptor *const sd_reader, const size_t length, msgpack_unpacked *ret_unpacked, char **const ret_buf) FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ARG(1) { - const uintmax_t initial_fpos = sd_reader->fpos; + const uintmax_t initial_fpos = sd_reader->bytes_read; char *const buf = xmalloc(length); const ShaDaReadResult fl_ret = fread_len(sd_reader, buf, length); @@ -2021,7 +1903,7 @@ static const char *shada_format_pfreed_entry(const PossiblyFreedShadaEntry pfs_e /// @param[in,out] ret_wms Location where results are saved. /// @param[out] packer MessagePack packer for entries which are not /// merged. -static inline ShaDaWriteResult shada_read_when_writing(ShaDaReadDef *const sd_reader, +static inline ShaDaWriteResult shada_read_when_writing(FileDescriptor *const sd_reader, const unsigned srni_flags, const size_t max_kbyte, WriteMergerState *const wms, @@ -2483,7 +2365,8 @@ static int hist_type2char(const int type) /// @param[in] sd_reader Structure containing file reader definition. If it is /// not NULL then contents of this file will be merged /// with current Neovim runtime. -static ShaDaWriteResult shada_write(FileDescriptor *const sd_writer, ShaDaReadDef *const sd_reader) +static ShaDaWriteResult shada_write(FileDescriptor *const sd_writer, + FileDescriptor *const sd_reader) FUNC_ATTR_NONNULL_ARG(1) { ShaDaWriteResult ret = kSDWriteSuccessful; @@ -2958,12 +2841,13 @@ int shada_write_file(const char *const file, bool nomerge) char *const fname = shada_filename(file); char *tempname = NULL; FileDescriptor sd_writer; - ShaDaReadDef sd_reader = { .close = NULL }; + FileDescriptor sd_reader; bool did_open_writer = false; + bool did_open_reader = false; if (!nomerge) { int error; - if ((error = open_shada_file_for_reading(fname, &sd_reader)) != 0) { + if ((error = file_open(&sd_reader, fname, kFileReadOnly, 0)) != 0) { if (error != UV_ENOENT) { semsg(_(SERR "System error while opening ShaDa file %s for reading " "to merge before writing it: %s"), @@ -2973,6 +2857,8 @@ int shada_write_file(const char *const file, bool nomerge) } nomerge = true; goto shada_write_file_nomerge; + } else { + did_open_reader = true; } tempname = modname(fname, ".tmp.a", false); if (tempname == NULL) { @@ -3001,8 +2887,9 @@ shada_write_file_open: {} fname); xfree(fname); xfree(tempname); - assert(sd_reader.close != NULL); - sd_reader.close(&sd_reader); + if (did_open_reader) { + close_file(&sd_reader); + } return FAIL; } (*wp)++; @@ -3047,8 +2934,8 @@ shada_write_file_nomerge: {} if (!did_open_writer) { xfree(fname); xfree(tempname); - if (sd_reader.cookie != NULL) { - sd_reader.close(&sd_reader); + if (did_open_reader) { + close_file(&sd_reader); } return FAIL; } @@ -3062,7 +2949,9 @@ shada_write_file_nomerge: {} const ShaDaWriteResult sw_ret = shada_write(&sd_writer, (nomerge ? NULL : &sd_reader)); assert(sw_ret != kSDWriteIgnError); if (!nomerge) { - sd_reader.close(&sd_reader); + if (did_open_reader) { + close_file(&sd_reader); + } bool did_remove = false; if (sw_ret == kSDWriteSuccessful) { FileInfo old_info; @@ -3235,18 +3124,18 @@ static inline uint64_t be64toh(uint64_t big_endian_64_bits) /// @return kSDReadStatusSuccess if everything was OK, kSDReadStatusNotShaDa if /// there were not enough bytes to read or kSDReadStatusReadError if /// there was some error while reading. -static ShaDaReadResult fread_len(ShaDaReadDef *const sd_reader, char *const buffer, +static ShaDaReadResult fread_len(FileDescriptor *const sd_reader, char *const buffer, const size_t length) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - const ptrdiff_t read_bytes = sd_reader->read(sd_reader, buffer, length); + const ptrdiff_t read_bytes = file_read(sd_reader, buffer, length); + if (read_bytes < 0) { + semsg(_(SERR "System error while reading ShaDa file: %s"), + os_strerror((int)read_bytes)); + return kSDReadStatusReadError; + } if (read_bytes != (ptrdiff_t)length) { - if (sd_reader->error != NULL) { - semsg(_(SERR "System error while reading ShaDa file: %s"), - sd_reader->error); - return kSDReadStatusReadError; - } semsg(_(RCERR "Error while reading ShaDa file: " "last entry specified that it occupies %" PRIu64 " bytes, " "but file ended earlier"), @@ -3272,26 +3161,32 @@ static ShaDaReadResult fread_len(ShaDaReadDef *const sd_reader, char *const buff /// @return kSDReadStatusSuccess if reading was successful, /// kSDReadStatusNotShaDa if there were not enough bytes to read or /// kSDReadStatusReadError if reading failed for whatever reason. -static ShaDaReadResult msgpack_read_uint64(ShaDaReadDef *const sd_reader, const int first_char, +/// kSDReadStatusFinished if eof and that was allowed +static ShaDaReadResult msgpack_read_uint64(FileDescriptor *const sd_reader, bool allow_eof, uint64_t *const result) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - const uintmax_t fpos = sd_reader->fpos - 1; - - if (first_char == EOF) { - if (sd_reader->error) { - semsg(_(SERR "System error while reading integer from ShaDa file: %s"), - sd_reader->error); - return kSDReadStatusReadError; - } else if (sd_reader->eof) { - semsg(_(RCERR "Error while reading ShaDa file: " - "expected positive integer at position %" PRIu64 - ", but got nothing"), - (uint64_t)fpos); - return kSDReadStatusNotShaDa; + const uintmax_t fpos = sd_reader->bytes_read; + + uint8_t ret; + ptrdiff_t read_bytes = file_read(sd_reader, (char *)&ret, 1); + + if (read_bytes < 0) { + semsg(_(SERR "System error while reading integer from ShaDa file: %s"), + os_strerror((int)read_bytes)); + return kSDReadStatusReadError; + } else if (read_bytes == 0) { + if (allow_eof && file_eof(sd_reader)) { + return kSDReadStatusFinished; } + semsg(_(RCERR "Error while reading ShaDa file: " + "expected positive integer at position %" PRIu64 + ", but got nothing"), + (uint64_t)fpos); + return kSDReadStatusNotShaDa; } + int first_char = (int)ret; if (~first_char & 0x80) { // Positive fixnum *result = (uint64_t)((uint8_t)first_char); @@ -3465,8 +3360,9 @@ static ShaDaReadResult msgpack_read_uint64(ShaDaReadDef *const sd_reader, const /// greater then given. /// /// @return Any value from ShaDaReadResult enum. -static ShaDaReadResult shada_read_next_item(ShaDaReadDef *const sd_reader, ShadaEntry *const entry, - const unsigned flags, const size_t max_kbyte) +static ShaDaReadResult shada_read_next_item(FileDescriptor *const sd_reader, + ShadaEntry *const entry, const unsigned flags, + const size_t max_kbyte) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { ShaDaReadResult ret = kSDReadStatusMalformed; @@ -3476,7 +3372,7 @@ shada_read_next_item_start: // somebody calls goto shada_read_next_item_error before anything is set in // the switch. CLEAR_POINTER(entry); - if (sd_reader->eof) { + if (file_eof(sd_reader)) { return kSDReadStatusFinished; } @@ -3486,19 +3382,15 @@ shada_read_next_item_start: uint64_t timestamp_u64; uint64_t length_u64; - const uint64_t initial_fpos = (uint64_t)sd_reader->fpos; - const int first_char = read_char(sd_reader); - if (first_char == EOF && sd_reader->eof) { - return kSDReadStatusFinished; - } + const uint64_t initial_fpos = sd_reader->bytes_read; ShaDaReadResult mru_ret; - if (((mru_ret = msgpack_read_uint64(sd_reader, first_char, &type_u64)) + if (((mru_ret = msgpack_read_uint64(sd_reader, true, &type_u64)) != kSDReadStatusSuccess) - || ((mru_ret = msgpack_read_uint64(sd_reader, read_char(sd_reader), + || ((mru_ret = msgpack_read_uint64(sd_reader, false, ×tamp_u64)) != kSDReadStatusSuccess) - || ((mru_ret = msgpack_read_uint64(sd_reader, read_char(sd_reader), + || ((mru_ret = msgpack_read_uint64(sd_reader, false, &length_u64)) != kSDReadStatusSuccess)) { return mru_ret; @@ -4158,63 +4050,6 @@ void shada_encode_gvars(msgpack_sbuffer *const sbuf) } while (var_iter != NULL); } -/// Wrapper for reading from msgpack_sbuffer. -/// -/// @return number of bytes read. -static ptrdiff_t read_sbuf(ShaDaReadDef *const sd_reader, void *const dest, const size_t size) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT -{ - msgpack_sbuffer *sbuf = (msgpack_sbuffer *)sd_reader->cookie; - const uintmax_t bytes_read = MIN(size, sbuf->size - sd_reader->fpos); - if (bytes_read < size) { - sd_reader->eof = true; - } - memcpy(dest, sbuf->data + sd_reader->fpos, (size_t)bytes_read); - sd_reader->fpos += bytes_read; - return (ptrdiff_t)bytes_read; -} - -/// Wrapper for read that ignores bytes read from msgpack_sbuffer. -/// -/// Used for skipping. -/// -/// @param[in,out] sd_reader ShaDaReadDef with msgpack_sbuffer. -/// @param[in] offset Amount of bytes to skip. -/// -/// @return FAIL in case of failure, OK in case of success. May set -/// sd_reader->eof. -static int sd_sbuf_reader_skip_read(ShaDaReadDef *const sd_reader, const size_t offset) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT -{ - msgpack_sbuffer *sbuf = (msgpack_sbuffer *)sd_reader->cookie; - assert(sbuf->size >= sd_reader->fpos); - const uintmax_t skip_bytes = MIN(offset, sbuf->size - sd_reader->fpos); - if (skip_bytes < offset) { - sd_reader->eof = true; - return FAIL; - } - sd_reader->fpos += offset; - return OK; -} - -/// Prepare ShaDaReadDef with msgpack_sbuffer for reading. -/// -/// @param[in] sbuf msgpack_sbuffer to read from. -/// @param[out] sd_reader Location where reader structure will be saved. -static void open_shada_sbuf_for_reading(const msgpack_sbuffer *const sbuf, ShaDaReadDef *sd_reader) - FUNC_ATTR_NONNULL_ALL -{ - *sd_reader = (ShaDaReadDef) { - .read = &read_sbuf, - .close = NULL, - .skip = &sd_sbuf_reader_skip_read, - .error = NULL, - .eof = false, - .fpos = 0, - .cookie = (void *)sbuf, - }; -} - /// Read ShaDa from msgpack_sbuffer. /// /// @param[in] file msgpack_sbuffer to read from. @@ -4226,7 +4061,8 @@ void shada_read_sbuf(msgpack_sbuffer *const sbuf, const int flags) if (sbuf->data == NULL) { return; } - ShaDaReadDef sd_reader; - open_shada_sbuf_for_reading(sbuf, &sd_reader); + FileDescriptor sd_reader; + file_open_buffer(&sd_reader, sbuf->data, sbuf->size); shada_read(&sd_reader, flags); + close_file(&sd_reader); } -- cgit From 064483a2b4a3056baf8eee4424bb81127e531991 Mon Sep 17 00:00:00 2001 From: bfredl Date: Tue, 28 May 2024 12:52:24 +0200 Subject: refactor(fileio): use a linear buffer for FileDescriptor Using a ring buffer for buffered synchronous fileio is just unnecessary complexity. - when reading, we always consume the _entire_ buffer before getting into syscalls. Thus we reset the buffer to its initial position before when we actually read. - when writing and buffer is full, we always flush the entire buffer before starting to buffer again. So we can reset the buffer to its initial state. Also no static buffers are needed for writing and skipping. Needing an extra copy for each write completely defeated the purpose of a ring buffer (if there had been one) --- src/nvim/msgpack_rpc/packer.c | 1 - src/nvim/os/fileio.c | 274 +++++++++++++++++++++--------------------- src/nvim/os/fileio_defs.h | 9 +- src/nvim/rbuffer.c | 19 +-- 4 files changed, 144 insertions(+), 159 deletions(-) (limited to 'src') diff --git a/src/nvim/msgpack_rpc/packer.c b/src/nvim/msgpack_rpc/packer.c index cac68f76f0..9c0d2910fa 100644 --- a/src/nvim/msgpack_rpc/packer.c +++ b/src/nvim/msgpack_rpc/packer.c @@ -113,7 +113,6 @@ void mpack_handle(ObjectType type, handle_T handle, PackerBuffer *packer) mpack_w(&packer->ptr, 0xc7); mpack_w(&packer->ptr, packsize); mpack_w(&packer->ptr, exttype); - // check_buffer(packer); memcpy(packer->ptr, buf, (size_t)packsize); packer->ptr += packsize; } diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index e58eb96c2e..c87b2d359f 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -120,12 +120,9 @@ int file_open_fd(FileDescriptor *const ret_fp, const int fd, const int flags) assert(!ret_fp->wr || !ret_fp->non_blocking); ret_fp->fd = fd; ret_fp->eof = false; - ret_fp->rv = rbuffer_new(kRWBufferSize); - ret_fp->_error = 0; - if (ret_fp->wr) { - ret_fp->rv->data = ret_fp; - ret_fp->rv->full_cb = (rbuffer_callback)&file_rb_write_full_cb; - } + ret_fp->buffer = alloc_block(); + ret_fp->read_pos = ret_fp->buffer; + ret_fp->write_pos = ret_fp->buffer; ret_fp->bytes_read = 0; return 0; } @@ -148,8 +145,9 @@ void file_open_buffer(FileDescriptor *ret_fp, char *data, size_t len) ret_fp->non_blocking = false; ret_fp->fd = -1; ret_fp->eof = true; - ret_fp->rv = rbuffer_new_wrap_buf(data, len); - ret_fp->_error = 0; + ret_fp->buffer = NULL; // we don't take ownership + ret_fp->read_pos = data; + ret_fp->write_pos = data + len; ret_fp->bytes_read = 0; } @@ -163,36 +161,18 @@ int file_close(FileDescriptor *const fp, const bool do_fsync) FUNC_ATTR_NONNULL_ALL { if (fp->fd < 0) { - rbuffer_free(fp->rv); return 0; } const int flush_error = (do_fsync ? file_fsync(fp) : file_flush(fp)); const int close_error = os_close(fp->fd); - rbuffer_free(fp->rv); + free_block(fp->buffer); if (close_error != 0) { return close_error; } return flush_error; } -/// Flush file modifications to disk -/// -/// @param[in,out] fp File to work with. -/// -/// @return 0 or error code. -int file_flush(FileDescriptor *const fp) - FUNC_ATTR_NONNULL_ALL -{ - if (!fp->wr) { - return 0; - } - file_rb_write_full_cb(fp->rv, fp); - const int error = fp->_error; - fp->_error = 0; - return error; -} - /// Flush file modifications to disk and run fsync() /// /// @param[in,out] fp File to work with. @@ -218,36 +198,29 @@ int file_fsync(FileDescriptor *const fp) return 0; } -/// Buffer used for writing -/// -/// Like IObuff, but allows file_\* callers not to care about spoiling it. -static char writebuf[kRWBufferSize]; - -/// Function run when RBuffer is full when writing to a file -/// -/// Actually does writing to the file, may also be invoked directly. +/// Flush file modifications to disk /// -/// @param[in,out] rv RBuffer instance used. /// @param[in,out] fp File to work with. -static void file_rb_write_full_cb(RBuffer *const rv, void *const fp_in) +/// +/// @return 0 or error code. +int file_flush(FileDescriptor *fp) FUNC_ATTR_NONNULL_ALL { - FileDescriptor *const fp = fp_in; - assert(fp->wr); - assert(rv->data == (void *)fp); - if (rbuffer_size(rv) == 0) { - return; + if (!fp->wr) { + return 0; + } + + ptrdiff_t to_write = fp->write_pos - fp->read_pos; + if (to_write == 0) { + return 0; } - const size_t read_bytes = rbuffer_read(rv, writebuf, kRWBufferSize); - const ptrdiff_t wres = os_write(fp->fd, writebuf, read_bytes, + const ptrdiff_t wres = os_write(fp->fd, fp->read_pos, (size_t)to_write, fp->non_blocking); - if (wres != (ptrdiff_t)read_bytes) { - if (wres >= 0) { - fp->_error = UV_EIO; - } else { - fp->_error = (int)wres; - } + fp->read_pos = fp->write_pos = fp->buffer; + if (wres != to_write) { + return (wres >= 0) ? UV_EIO : (int)wres; } + return 0; } /// Read from file @@ -262,77 +235,78 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, const size_t FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { assert(!fp->wr); - char *buf = ret_buf; - size_t read_remaining = size; - RBuffer *const rv = fp->rv; + size_t from_buffer = MIN((size_t)(fp->write_pos - fp->read_pos), size); + memcpy(ret_buf, fp->read_pos, from_buffer); + + char *buf = ret_buf + from_buffer; + size_t read_remaining = size - from_buffer; + if (!read_remaining) { + fp->bytes_read += from_buffer; + fp->read_pos += from_buffer; + return (ptrdiff_t)from_buffer; + } + + // at this point, we have consumed all of an existing buffer. restart from the beginning + fp->read_pos = fp->write_pos = fp->buffer; + +#ifdef HAVE_READV bool called_read = false; while (read_remaining) { - const size_t rv_size = rbuffer_size(rv); - if (rv_size > 0) { - const size_t rsize = rbuffer_read(rv, buf, MIN(rv_size, read_remaining)); - buf += rsize; - read_remaining -= rsize; - } - if (fp->eof - // Allow only at most one os_read[v] call. - || (called_read && fp->non_blocking)) { + // Allow only at most one os_read[v] call. + if (fp->eof || (called_read && fp->non_blocking)) { break; } - if (read_remaining) { - assert(rbuffer_size(rv) == 0); - rbuffer_reset(rv); -#ifdef HAVE_READV - // If there is readv() syscall, then take an opportunity to populate - // both target buffer and RBuffer at once, … - size_t write_count; - struct iovec iov[] = { - { .iov_base = buf, .iov_len = read_remaining }, - { .iov_base = rbuffer_write_ptr(rv, &write_count), - .iov_len = kRWBufferSize }, - }; - assert(write_count == kRWBufferSize); - const ptrdiff_t r_ret = os_readv(fp->fd, &fp->eof, iov, - ARRAY_SIZE(iov), fp->non_blocking); - if (r_ret > 0) { - if (r_ret > (ptrdiff_t)read_remaining) { - rbuffer_produced(rv, (size_t)(r_ret - (ptrdiff_t)read_remaining)); - read_remaining = 0; - } else { - buf += (size_t)r_ret; - read_remaining -= (size_t)r_ret; - } - } else if (r_ret < 0) { - return r_ret; - } -#else - if (read_remaining >= kRWBufferSize) { - // …otherwise leave RBuffer empty and populate only target buffer, - // because filtering information through rbuffer will be more syscalls. - const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof, buf, read_remaining, - fp->non_blocking); - if (r_ret >= 0) { - read_remaining -= (size_t)r_ret; - fp->bytes_read += (size - read_remaining); - return (ptrdiff_t)(size - read_remaining); - } else if (r_ret < 0) { - return r_ret; - } + // If there is readv() syscall, then take an opportunity to populate + // both target buffer and RBuffer at once, … + struct iovec iov[] = { + { .iov_base = buf, .iov_len = read_remaining }, + { .iov_base = fp->write_pos, + .iov_len = ARENA_BLOCK_SIZE }, + }; + const ptrdiff_t r_ret = os_readv(fp->fd, &fp->eof, iov, + ARRAY_SIZE(iov), fp->non_blocking); + if (r_ret > 0) { + if (r_ret > (ptrdiff_t)read_remaining) { + fp->write_pos += (size_t)(r_ret - (ptrdiff_t)read_remaining); + read_remaining = 0; } else { - size_t write_count; - const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof, - rbuffer_write_ptr(rv, &write_count), - kRWBufferSize, fp->non_blocking); - assert(write_count == kRWBufferSize); - if (r_ret > 0) { - rbuffer_produced(rv, (size_t)r_ret); - } else if (r_ret < 0) { - return r_ret; - } + buf += r_ret; + read_remaining -= (size_t)r_ret; } -#endif - called_read = true; + } else if (r_ret < 0) { + return r_ret; + } + called_read = true; + } +#else + if (fp->eof) { + // already eof, cannot read more + } else if (read_remaining >= ARENA_BLOCK_SIZE) { + // …otherwise leave fp->buffer empty and populate only target buffer, + // because filtering information through rbuffer will be more syscalls. + const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof, buf, read_remaining, + fp->non_blocking); + if (r_ret >= 0) { + read_remaining -= (size_t)r_ret; + } else if (r_ret < 0) { + return r_ret; + } + } else { + const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof, + fp->write_pos, + ARENA_BLOCK_SIZE, fp->non_blocking); + if (r_ret < 0) { + return r_ret; + } else { + fp->write_pos += r_ret; + size_t to_copy = MIN((size_t)r_ret, read_remaining); + memcpy(ret_buf, fp->read_pos, to_copy); + fp->read_pos += to_copy; + read_remaining -= to_copy; } } +#endif + fp->bytes_read += (size - read_remaining); return (ptrdiff_t)(size - read_remaining); } @@ -348,40 +322,68 @@ ptrdiff_t file_write(FileDescriptor *const fp, const char *const buf, const size FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ARG(1) { assert(fp->wr); - const size_t written = rbuffer_write(fp->rv, buf, size); - if (fp->_error != 0) { - const int error = fp->_error; - fp->_error = 0; - return error; - } else if (written != size) { - return UV_EIO; + ptrdiff_t space = (fp->buffer + ARENA_BLOCK_SIZE) - fp->write_pos; + // includes the trivial case of size==0 + if (size < (size_t)space) { + memcpy(fp->write_pos, buf, size); + fp->write_pos += size; + return (ptrdiff_t)size; + } + + // TODO(bfredl): just as for reading, use iovec to combine fp->buffer with buf + int status = file_flush(fp); + if (status < 0) { + return status; + } + + if (size < ARENA_BLOCK_SIZE) { + memcpy(fp->write_pos, buf, size); + fp->write_pos += size; + return (ptrdiff_t)size; } - return (ptrdiff_t)written; -} -/// Buffer used for skipping. Its contents is undefined and should never be -/// used. -static char skipbuf[kRWBufferSize]; + const ptrdiff_t wres = os_write(fp->fd, buf, size, + fp->non_blocking); + return (wres != (ptrdiff_t)size && wres >= 0) ? UV_EIO : wres; +} /// Skip some bytes /// /// This is like `fseek(fp, size, SEEK_CUR)`, but actual implementation simply -/// reads to a buffer and discards the result. +/// reads to the buffer and discards the result. ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size) FUNC_ATTR_NONNULL_ALL { assert(!fp->wr); - size_t read_bytes = 0; - do { - const ptrdiff_t new_read_bytes = - file_read(fp, skipbuf, MIN(size - read_bytes, sizeof(skipbuf))); - if (new_read_bytes < 0) { - return new_read_bytes; - } else if (new_read_bytes == 0) { + size_t from_buffer = MIN((size_t)(fp->write_pos - fp->read_pos), size); + size_t skip_remaining = size - from_buffer; + if (skip_remaining == 0) { + fp->read_pos += from_buffer; + fp->bytes_read += from_buffer; + return (ptrdiff_t)from_buffer; + } + + fp->read_pos = fp->write_pos = fp->buffer; + bool called_read = false; + while (skip_remaining > 0) { + // Allow only at most one os_read[v] call. + if (fp->eof || (called_read && fp->non_blocking)) { break; } - read_bytes += (size_t)new_read_bytes; - } while (read_bytes < size && !file_eof(fp)); + const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof, fp->buffer, ARENA_BLOCK_SIZE, + fp->non_blocking); + if (r_ret < 0) { + return r_ret; + } else if ((size_t)r_ret > skip_remaining) { + fp->read_pos = fp->buffer + skip_remaining; + fp->write_pos = fp->buffer + r_ret; + fp->bytes_read += size; + return (ptrdiff_t)size; + } + skip_remaining -= (size_t)r_ret; + called_read = true; + } - return (ptrdiff_t)read_bytes; + fp->bytes_read += size - skip_remaining; + return (ptrdiff_t)(size - skip_remaining); } diff --git a/src/nvim/os/fileio_defs.h b/src/nvim/os/fileio_defs.h index 10277d2a7a..63b6f51c17 100644 --- a/src/nvim/os/fileio_defs.h +++ b/src/nvim/os/fileio_defs.h @@ -8,9 +8,10 @@ /// Structure used to read from/write to file typedef struct { - int fd; ///< File descriptor. - int _error; ///< Error code for use with RBuffer callbacks or zero. - RBuffer *rv; ///< Read or write buffer. + int fd; ///< File descriptor. Can be -1 if no backing file (file_open_buffer) + char *buffer; ///< Read or write buffer. always ARENA_BLOCK_SIZE if allocated + char *read_pos; ///< read position in buffer + char *write_pos; ///< write position in buffer bool wr; ///< True if file is in write mode. bool eof; ///< True if end of file was encountered. bool non_blocking; ///< True if EAGAIN should not restart syscalls. @@ -28,7 +29,7 @@ static inline bool file_eof(const FileDescriptor *fp) /// performed. static inline bool f