Followup to #28515:
Rename the static os_homedir() to os_uv_homedir() to emphasize that it
is a wrapper around a libuv function.
Add the function os_get_homedir() to os/env.c to return the cached
homedir value as a const. Must be called after homedir is initialized or
it fails.
The difference between this function and the static os_uv_homedir() is
that the latter gets the homedir from libuv and is used to initialize
homedir in init_homedir(), while os_get_homedir() just returns homedir
as a const if it's initialized and is public.
Use the os_get_homedir() accessor for ~/ expansion on Windows to make
the code more concise.
Add a Windows section to main_spec.lua with tests for expanding ~/ and
~\ prefixes for files passed in on the command-line.
Signed-off-by: Rafael Kitover <rkitover@gmail.com>
The latter was mostly relevant with the past char_u madness.
NOTE: STRCAT also functioned as a counterfeit "NOLINT" for clint
apparently. But NOLINT-ing every usecase is just the same as disabling
the check entirely.
If you like it you shouldn't put a ring on it.
This is what _every_ consumer of RStream used anyway, either by calling
rbuffer_reset, or rbuffer_consumed_compact (same as rbuffer_reset
without needing a scratch buffer), or by consuming everything in
each stream_read_cb call directly.
Since paste data is handled via a separate channel, the data processed via `input_buffer` is typically just explicit keys as typed in by the user. Therefore it should be fine to use `memmove()` to always put the remaining data in front when refilling the buffer.
This is a structural refactor with no logical changes, yet. Done in
preparation for simplifying rstream/rbuffer which will require more
state inline in RStream.
The initial idea was to have RStream and WStream as sub-types
symetrically but that doesn't work, as sockets are both reading and
writing. Also there is very little write-specific state to start with,
so the benefit of a separate WStream struct is a lot smaller. Just
document what fields in `Stream` are write specific.
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)
`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.
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)
Use uv_fs_realpath() instead.
It seems that uv_fs_realpath() has some problems on non-Linux platforms:
- macOS and other BSDs: this function will fail with UV_ELOOP if more
than 32 symlinks are found while resolving the given path. This limit
is hardcoded and cannot be sidestepped.
- Windows: while this function works in the common case, there are a
number of corner cases where it doesn't:
- Paths in ramdisk volumes created by tools which sidestep the Volume
Manager (such as ImDisk) cannot be resolved.
- Inconsistent casing when using drive letters.
- Resolved path bypasses subst'd drives.
Ref: https://docs.libuv.org/en/v1.x/fs.html#c.uv_fs_realpath
I don't know if the old implementation that uses uv_chdir() and uv_cwd()
also suffers from the same problems.
- For the ELOOP case, chdir() seems to have the same limitations.
- On Windows, Vim doesn't use anything like chdir() either. It uses
_wfullpath(), while libuv uses GetFinalPathNameByHandleW().
There's no "rule" or bad practice or whatever that says we cannot
generate c files. it is is just that we have ~20 generated headers
and ~2 generated sources and there is nothing in these two generated
source files which sets them aparts. Lua bindings are not different from
rpc bindings, and pathdef is not different from versiondef.
So to simplify build logic and ease the future port to build.zig,
streamline the build to only have generated headers, no direct generated
.c files.
Also "nlua_add_api_functions" had its prototype duplicated twice which
defeated the point of having mandatory prototypes (one source of truth).
Problem: Using xstrlcpy() when the exact length of the string to be
copied is known is not ideal because it requires adding 1 to
the length and an unnecessary strlen().
Solution: Add xmemcpyz() and use it in place of such xstrlcpy() calls.
If the filename passed to vim_FullName() is a relative directory, and
does not exist, it is appended to the current working directory. Since
the return value of append_path() was ignored, and if the buffer length
was too small to fit getcwd() + dirname(filename), it would still try to
append the basename(filename).
This was manifesting as a failure in test/unit/path_spec.lua in:
itp('fails and uses filename if given filename contains non-existing directory', ..
This failure occurs when running the tests from directory with a short
path such as: /work/src/nv
test/unit/path_spec.lua:420: Expected objects to be the same.
Passed in:
(string) '/work/src/nv/test.file'
Expected:
(string) 'non_existing_dir/test.file'
This return value for the second call to append_path() to append
basename(filename) was checked, and this is where it would fail for
normal / longer getcwd()s.
Problem: Dialog for file changed outside of Vim not tested.
Solution: Add a test. Move FileChangedShell test. Add 'L' flag to
feedkeys().
5e66b42aae
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem: problem with writing extended attributes on failure
Solution: Change return type to ssize_t and check listxattr's return
value correctly on failure (Paul Tagliamonte)
The existing logic will return when the listxattr call returns with the
errno set to ENOTSUP (or a size of 0 bytes), without checking to see if
listxattr actually failed. listxattr can fail with at least E2BIG,
ENOTSUP, ERANGE, or anything that `stat(2)` can fail with (in my case;
ENOENT from stat).
The returned size is stored to a size_t, but the return type is a
ssize_t. On failure, listxattr returns -1, which will get translated to
size_t's MAX. If the listxattr call failed with anything other than
ENOTSUP, this triggers a request for size_t MAX bytes.
This means that, if the listxattr call fails with anything other than
ENOTSUP on save, vim will error with
`E342: Out of memory! (allocating 18446744073709551615 bytes)`
(keen observers will note 18446744073709551615 is 0xffffffffffffffff)
In reality, this is likely masking a different (usually filesystem?)
error -- but at least it's an error being pushed to the user now, and we
don't try to allocate size_t MAX bytes.
I've opted to change the type that we store listxattr to from size_t to
ssize_t, to match listxattr(2)'s signature, and to check for the -1
return value. Additionally, I've removed the errno check -- if we get a
listxattr failure for any reason, we may as well bail without trying;
it's not like we can even recover.
closes: vim/vim#1416914759ded57
Co-authored-by: Paul R. Tagliamonte <paultag@gmail.com>
Problem: expand() removes a slash after an environment variable that
ends with a colon on Windows.
Solution: Check the correct char for a colon (zeertzjq)
closes: vim/vim#14161
Note: Vim still removes the path-separator at the end, if another path separator
follows directly after it, e.g. on:
```
echo $FOO='/usr/'
echo expand('$FOO/bar') == '/usr/bar'
```
see:
,----[ misc1.c:1630 ]
| // if var[] ends in a path separator and tail[] starts
| // with it, skip a character
| if (after_pathsep(dst, dst + c)
| #if defined(BACKSLASH_IN_FILENAME) || defined(AMIGA)
| && (dst == save_dst || dst[-1] != ':')
| #endif
| && vim_ispathsep(*tail))
| ++tail;
`----
13a014452a
Cherry-pick test_expand.vim change from patch 9.0.1257.
Problem: Various warnings from clang on MS-Windows.
Solution: Avoid the warnings. (Yegappan Lakshmanan, closesvim/vim#10553)
a34b4460c2
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
On exit, pty_process_close() may be called after pty_process_finish1()
but before start_wait_eof_timer(), in which case the timer shouldn't be
started because pty_process_close() has already closed it.
Then we can just load metadata in C as a single msgpack blob. Which also
can be used directly as binarly data, instead of first unpacking all the
functions and ui_events metadata to immediately pack it again, which was
a bit of a silly walk (and one extra usecase of `msgpack_rpc_from_object`
which will get yak shaved in the next PR)
Functions like file_open_new() and file_open_fd_new() which just is a
wrapper around the real functions but with an extra xmalloc/xfree around
is an anti-pattern. If the caller really needs to allocate a
FileDescriptor as a heap object, it can do that directly.
FileDescriptor by itself is pretty much a pointer, or rather two:
the OS fd index and a pointer to a buffer. So most of the time an extra
pointer layer is just wasteful.
In the case of scriptin[curscript] in getchar.c, curscript used
to mean in practice:
N+1 open scripts when curscript>0
zero or one open scripts when curscript==0
Which means scriptin[0] had to be compared to NULL to disambiguate the
curscript=0 case.
Instead, use curscript==-1 to mean that are no script,
then all pointer comparisons dissappear and we can just use an array of
structs without extra pointers.
os_getenv("FOO") caches the result when $FOO is set to something
non-empty. However, when $FOO was not set, every new call to
os_getenv("FOO") would allocate a temporary scratch buffer to
immediately throw away.
This has an huge impact e.g. on logging which depends on potentially
non-set env vars.
Problem: Code is indented more than necessary.
Solution: Use an early return where it makes sense. (Yegappan Lakshmanan,
closesvim/vim#11813)
e857598896
Skip list_alloc_with_items().
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
When an embedded Nvim instance changes its current directory a "chdir"
UI event is emitted. Attached UIs can use this information however they
wish. In the TUI it is used to synchronize the cwd of the TUI process
with the cwd of the embedded Nvim process.
Remove `export` pramgas from defs headers as it causes IWYU to believe
that the definitions from the defs headers comes from main header, which
is not what we really want.