nvim_buf_*() and unloaded buffers #8660

phodge · 2018-06-29T07:48:28Z

Proposed solution for #7688 and #5534. Changes in this PR:

nvim_buf_get_lines() will now always return an empty list for an unloaded buffer, instead of a list containing a single empty string.
nvim_buf_line_count() will now always return 0 for an unloaded buffer, instead of 1.
nvim_buf_is_loaded() has been added to check if a buffer is loaded
help text updates for the above
extra unit tests for the above

bfredl · 2018-06-29T09:19:47Z

runtime/doc/api.txt

@@ -847,7 +847,8 @@ nvim_buf_line_count({buffer})                          *nvim_buf_line_count()*
                    {buffer}  Buffer handle

                Return:~
-                    Line count
+                    Line count, or `0` if the buffer has been unloaded (see


This section is autogenerated, please change the docstrings in api/buf.c only

What command can I run to regenerate the api docs locally?

python3 scripts/gen_api_vimdoc.py

justinmk · 2018-07-02T08:59:41Z

src/nvim/api/buffer.c

 ///
 /// @param buffer Buffer handle
-/// @return true if the buffer is valid, false otherwise
+/// @return true if the buffer is valid, false otherwise.
 Boolean nvim_buf_is_valid(Buffer buffer)


Just noticed that this returns true for 0.

Yeah all of the nvim_buf_*() functions accept a buffer id of 0, which is interpreted as "the current buffer". This feature is used in the lua tests.

Yes but does it make sense for this function?

@justinmk what would 0 mean instead in this case? And why is that better than curbuf?

@bfredl I don't understand your question about curbuf.

I'm thinking nvim_buf_is_valid(0) should return false, because nvim_buf_is_valid is for checking validity of buffer handles, and 0 is not a buffer handle. 0 makes sense for other buffer-related functions, but not this one.

But 0 is a valid buffer handle, and it means curbuf. Why should it mean something different for this function?

It's a special value that only is relevant in the context of a buffer-related function. If someone uses nvim_buf_is_valid to validate a loop or something like that, then nvim_buf_is_valid (0) => true might be unhelpful.

And exactly how is this function not even related to buffers? I'm not saying true is obviously objectively better a priori, just that if we are introducing a special case, and a change of behaviour of existing input at that, I want a stronger argument than "maybe false is more useful in a some more situations". After all, clients can already special case 0 as they want, the only functional effect of changing this now is potentially breaking code that already uses this function and didn't anticipate this change.

If neovim was ever refactored such that the curbuf global variable could be NULL, perhaps during early startup when neovim hasn't had a chance to open a file yet, then nvim_buf_is_valid(0) would be the logical way to ask neovim "Is there a current buffer yet?".

@phodge In that hypothetical situation: yes. Then most other nvim_buf_ functions would also unconditionally throw an error in that situation. (if we want such a situation to be exposed to local and remote user code is of course a different question, the programming model is simpler as long as we can avoid it)

phodge · 2018-07-04T06:40:51Z

The QuickBuild error (from inccommand tests) appears to be unrelated?

KillTheMule · 2018-07-04T07:17:23Z

That's the "inccomand during term activity" test, that's known to be flakey.

bfredl · 2018-07-04T08:01:27Z

src/nvim/api/buffer.c

+Boolean nvim_buf_is_loaded(Buffer buffer)
+  FUNC_API_SINCE(5)
+{
+  Error stub = ERROR_INIT;


Maybe we should use Error *err here, then it would be possible to identify the three states: loaded, unloaded (thus loadable) and invalid in one RPC call.

My hesitation with returning an error for invalid buffers is that you'd now need to introduce error handling code (try/except in python or pcall() in lua) to use buf.is_loaded(). Which in turn means you can't write a one-liner like if buf.is_loaded() && buf.line_count() > 10:

That makes sense. We could add some more detailed state function later it need be.

(for posterity: just realized nvim_buf_line_count is already this function.)

phodge · 2018-07-05T20:50:21Z

@justinmk, @bfredl: I'd like to finalize this PR if possible, is there anything else that needs addressing?

phodge · 2018-07-05T21:23:37Z

I've updated the description to mention that #5534 will also be resolved by this PR.

bfredl · 2018-07-06T10:14:37Z

src/nvim/api/buffer.c

@@ -35,7 +35,8 @@
 ///
 /// @param buffer   Buffer handle
 /// @param[out] err Error details, if any
-/// @return Line count
+/// @return Line count, or \`0` if the buffer has been unloaded (see
+///         |nvim_buf_is_loaded()|).


perhaps this could be formulated to make it clear if nvim_buf_line_count is already used, then nvim_buf_is_loaded need not also be used. The later is only referenced because shared info of all buffer functions is placed there, ideally this information could be at |api-buffer| before any specific function (but the generation script maybe needs to be changed to not overwrite such a section).

I've updated the nvim_buf_is_loaded() docs to mention that you can also use nvim_buf_line_count(). I have also moved the comments on unloaded buffers to a new intro paragraph at the top of the api-buffer section.

bfredl · 2018-07-06T10:18:14Z

test/functional/api/buffer_spec.lua

+      eq(4, request('buffer_line_count', bufnr))
+      eq(4, request('nvim_buf_line_count', bufnr))
+      -- force unload the buffer (this will discard changes)
+      request('nvim_command','new')


use command('new')

bfredl · 2018-07-06T10:19:36Z

test/functional/api/buffer_spec.lua

+      request('nvim_command','new')
+      request('nvim_command', 'bunload! '..bufnr)
+      -- line count for an unloaded buffer should always be 0
+      eq(0, request('buffer_line_count', bufnr))


obsolete functions (or even just an obsolete name in this case) need not be tested in new tests, unless there is some edge case that affects the obsolete function or something (which is not here, this this just an alias)

bfredl · 2018-07-06T10:23:05Z

test/functional/api/buffer_spec.lua

+      command('bunload! '..bufnr)
+      -- attempting to get lines now always gives empty list
+      eq({}, buffer('get_lines', bufnr, 1, 3, 1))
+      eq({}, request('buffer_get_lines', bufnr, 1, 3, 1))


same here, buffer_get_lines is just an alias, while the even older buffer_get_line_range could be potentially interesting to test in some situation (but it is probably not worth the trouble either here).

bfredl · 2018-07-06T10:28:40Z

test/functional/api/buffer_spec.lua


+    it('line_count has defined behaviour for unloaded buffers', function()
+      -- we'll need to know our bufnr for when it gets unloaded
+      local bufnr = request('nvim_buf_get_number', 0)


nvim_buf_get_number is deprecated, nvim_get_current_buf is preferred for this, or just use curbuf() without args.

bfredl · 2018-07-06T10:44:14Z

Good code changes, just some minor quibbles about docs and tests. 👍

phodge · 2018-07-09T01:28:10Z

OK that should be everything

bfredl · 2018-07-09T07:59:03Z

test/functional/api/buffer_spec.lua

+      buffer('set_lines', bufnr, 0, -1, 1, {"line1", "line2", "line3", "line4"})
+      -- confirm that getting lines works
+      eq({"line2", "line3"}, buffer('get_lines', bufnr, 1, 3, 1))
+      eq({"line2", "line3"}, request('buffer_get_lines', bufnr, 1, 3, 1))


this line can be removed

bfredl · 2018-07-09T07:59:44Z

test/functional/api/buffer_spec.lua


+    it('line_count has defined behaviour for unloaded buffers', function()
+      -- we'll need to know our bufnr for when it gets unloaded
+      local bufnr = buffer('get_number', 0)


use nvim('get_current_buf') or curbuf() (one more case below)

bfredl · 2018-07-09T08:06:36Z

scripts/gen_api_vimdoc.py

    sep = '=' * text_width

    base = os.path.join(out_dir, 'xml')
    dom = minidom.parse(os.path.join(base, 'index.xml'))
+
+    # generate docs for section intros
+    for compound in dom.getElementsByTagName('compound'):


nice improvement!

bfredl · 2018-07-09T21:54:18Z

test/functional/api/buffer_spec.lua

@@ -38,7 +38,7 @@ describe('api/buf', function()

    it('line_count has defined behaviour for unloaded buffers', function()
      -- we'll need to know our bufnr for when it gets unloaded
-      local bufnr = buffer('get_number', 0)
+      local bufnr = curbuf('get_number')


I meant just curbuf() with no args.

I need the buffer number to use in command('bunload! '..bufnr), but I can't figure out how to get the buffer number from the handle returned by curbuf() with no args.

The handle returned by curbuf() is the buffer number.

bfredl · 2018-07-14T07:57:19Z

LGTM. Squash and rebase?

phodge · 2018-08-01T23:40:56Z

@bfredl: I've rebased into a smaller set of commits. I'm not 100% certain what's going on with the build there - but I'm pretty sure the errors are unrelated. I think it's OK to merge now, but I don't have access to do that myself.

bfredl · 2018-08-02T08:43:28Z

Failures look unrelated, I restarted the failed builds.

bfredl · 2018-08-02T11:29:24Z

merged, thanks!

Besides the "visible" improvements, this release features numerous internal improvements to the UI/screen code and test infrastructure. Numerous patches were merged from Vim, which are not mentioned below. FEATURES: 07ad5d7 clipboard: Support custom VimL functions neovim#9304 725da1f neovim#9401 win/TUI: Improve terminal/console support 7a8dadb neovim#9077 startup: Use $XDG_CONFIG_DIRS/nvim/sysinit.vim if it exists feec926 neovim#9299 support <cmd> mapping in more places 0653ed6 neovim#9028 diff/highlight: Show underline for low-priority CursorLine bddcbbb signs: Add "numhl" argument neovim#9113 05f9c7c clipboard: support Wayland (neovim#9230) 14ae394 neovim#9052 TUI: add support for undercurl and underline color 4fa3492 neovim#9023 man.vim: soft (dynamic) wrap neovim#9023 API: 8b39e4e neovim#6920 API: implement object namespaces b1aaa0a API: Implement nvim_win_set_buf() neovim#9100 8de87c7 neovim#8180 API: virtual text annotations (nvim_buf_set_virtual_text) 2b9fc9a neovim#8660 API: add nvim_buf_is_loaded() API: buf_get_lines, buf_line_count handle unloaded buffers 88f77c2 API: nvim_buf_get_offset_for_line 94841e5 API/UI: neovim#8221 ext_newgrid, ext_hlstate (use line-based rather than char-based updates) UI b5cfac0 neovim#8806 TUI: use BCE again more often, (smoother resizes/scrolling) 77b5e9a neovim#9315 screen: add missing status redraw when redraw_later(CLEAR) was used 5f15788 TUI: clip invalid regions on resize (neovim#8779), fixes neovim#8774 c936ae0 neovim#9193 TUI: improvements for scrolling and clearing f204274 neovim#9143 UI: disable clearing almost everywhere f4b2b66 neovim#9079 TUI: always use safe cursor movement after resize d36afaf neovim#9211 ui_options: also send when starting or from OptionSet 67f80d4 TUI: Avoid reset_cursor_color in old VTE neovim#9191 e55ebae neovim#9021 don't erase screen on `:hi Normal` during startup c5790d9 neovim#8915 TUI: Hint wrapped lines to terminals. FIXES: 231de72 RPC: turn errors from async calls into notifications 907ad92 TUI: Restore terminal title via "title stacking" (neovim#9407) cb76a8a genappimage: Unset $ARGV0 at invocation neovim#9376 b48efd9 neovim#9347 TUI: FreeBSD: Improve support for BSD vt console c16529a TUI: Konsole 18.07.70 supports DECSCUSR (neovim#9364) aec096f os/lang: use the correct LC_NUMERIC also for OS X 5fee0be provider: improve error message (neovim#9344) 3c42d7a TUI: alacritty supports set_cursor_color neovim#9353 7bff9a5 TUI: Alacritty supports DECSCUSR (neovim#9048) 57acfce macOS: infer primary language if $LANG is empty neovim#9345 bc132ae runtime/syntax: Fix highlighting of augroup contents (neovim#9328) 715fdfe neovim#9297 VimL/confirm(): Show dialog even if :silent 799d9c3 clipboard: Prefer xclip (neovim#9302) 6dae777 provider/nodejs: fix npm,yarn detection 16bc1e9 neovim#9218 channel: avoid buffering output when only terminal and no callbacks are active 72fecad neovim#8804 Fix crash in lang_init() on macOS if lang_region = NULL d581398 ruby: detect rbenv shims for other versions (neovim#8733) e568ac7 neovim#9123 third-party/unibilium: Fix parsing of extended capability entries c4c74c3 jobstart(): Fix hang on non-executable cwd neovim#9204 1cf50cb provider/nodejs: Simultaneously query npm and yarn neovim#9054 6c496db undo: Fix infinite loop if undo_read_byte returns EOF neovim#2880 f8f8357 neovim#9034 'swapfile: always show dialog' CHANGES: c236e80 neovim#9024 --embed: wait for UI unless --headless 180b50d neovim#9248 python: 'neovim' module was renamed to 'pynvim' 2000b6a neovim#8589 VimL: Remove legacy aliases "v:errmsg", "v:shell_error", "v:this_session" deb18a0 defaults: background=dark neovim#2894 (neovim#9205) c1187d4 defaults: win: 'shellpipe' for cmd.exe (neovim#8827)

bfredl reviewed Jun 29, 2018

View reviewed changes

justinmk reviewed Jul 2, 2018

View reviewed changes

bfredl reviewed Jul 4, 2018

View reviewed changes

phodge mentioned this pull request Jul 5, 2018

nvim_buf_get_lines() only works when buffer has a window open #5534

Closed

bfredl reviewed Jul 6, 2018

View reviewed changes

bfredl reviewed Jul 9, 2018

View reviewed changes

phodge added 7 commits July 25, 2018 14:09

cmake: bump API version

ae52170

API: buf_get_lines, buf_line_count handle unloaded buffers neovim#7688

40e2d6b

API: add nvim_buf_is_loaded() neovim#7688

8ff0872

API: update docs WRT behaviours/methods for unloaded buffers neovim#7688

10bb11e

functests: tests related to operations on unloaded buffers neovim#7688

b53b621

DOC: add support for intro sections in api docs

7ab27ee

DOC: regenerate api docs

ea2e8f4

phodge force-pushed the 7688-nvim-buf-lines-should-return-empty-list-for-unloaded-buffer branch from f16e10e to ea2e8f4 Compare July 25, 2018 05:28

bfredl merged commit 2b9fc9a into neovim:master Aug 2, 2018

justinmk mentioned this pull request Aug 2, 2018

nvim_buf_get_lines should return [] for unloaded buffer #7688

Closed

justinmk mentioned this pull request Oct 4, 2018

[RFC] debug: spawn child process with --inspect-brk neovim/node-client#95

Merged

dlants mentioned this pull request Jul 11, 2025

fixup get file and replace timing issue dlants/magenta.nvim#135

Merged

Uh oh!

nvim_buf_*() and unloaded buffers #8660

nvim_buf_*() and unloaded buffers #8660

Uh oh!

Conversation

phodge commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfredl Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfredl Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phodge commented Jul 4, 2018

Uh oh!

KillTheMule commented Jul 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phodge commented Jul 5, 2018

Uh oh!

phodge commented Jul 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfredl commented Jul 6, 2018

Uh oh!

phodge commented Jul 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phodge commented Jun 29, 2018 •

edited

Loading

justinmk Jul 4, 2018 •

edited

Loading

bfredl Jul 4, 2018 •

edited

Loading

bfredl Jul 4, 2018 •

edited

Loading