PR #4028 moved the check for autocomplete from action autocomplete to
the buffer one, which caused a few regression for upstream callers
(commandbar autocomplete & plugins that use buffer autocomplete)
that use the buffer autocomplete.
This commit restores pre #4028 behavior by adding a callback variant
for buffer autocomplete functions while keeping the multi-cursor
autocomplete function.
Otherwise we can't identify if we have the same file open multiple times via
different symlinks.
The test must be adapted to resolve symlinks in `findBuffer()`.
Otherwise we unnecessarily serialize the shared buffer every time when closing
a bufpane with this buffer, so every such serialize overwrites the previous one,
thus only the last serialize (when closing the last instance of the buffer, i.e.
when actually closing the file, i.e. when the buffer is not shared anymore) will
be used anyway.
Verify the sudo command is available via exec.LookPath() before
creating the backup in safeWrite(). This avoids bogus backup files
when the sudo binary is not found.
Signed-off-by: Nilton Perim Neto <[email protected]>
When saving a protected file, micro spawned two sudo dd subprocesses
concurrently: a truncation dd and a write dd. Both competed for the
TTY to prompt for the sudo password, racing on TCGETS2/TCSETS2 ioctls
and deadlocking stdin routing. This strictly fails with sudo-rs, which
enforces auditable escalation boundaries.
Fix by removing notrunc and the separate truncation subprocess entirely.
A single dd subprocess now handles truncation and write in one shot,
making the race impossible by construction. The conv=fsync guarantee is
preserved on platforms that support it.
Move writeBackup() before openFile() in safeWrite() for the sudo path
so the backup always exists before dd truncates the file. If openFile()
fails after the backup is created, return OverwriteError so the user
knows their data is safe in the backup. The non-sudo path is unchanged.
Link: https://github.com/micro-editor/micro/issues/4050
Signed-off-by: Nilton Perim Neto <[email protected]>
Micro always saves the undo stack information into the serialized buffer
file as long as either `saveundo` or `savecursor` is enabled. Whereas in
the case when only `savecursor` is enabled, while `saveundo` is
disabled, this information is not used afterwards, so it only wastes the
disk space in `~/.config/micro/buffers`. (And given that currently micro
never automatically removes any serialized buffer files, it may
significantly contribute to the overall ever growing size of the
`~/.config/micro/buffers` directory.)
So avoid saving the undo info if `saveundo` is disabled. This makes the
size of each serialized buffer file with savecursor=on saveundo=off
small and predictable, e.g. around 600 bytes in my observations (whereas
without this fix, it may grow indefinitely big, depending on the number
of modifications the user made before saving the file).
Since full escaped backup paths can become longer than the maximum filename size
and hashed filenames cannot be restored it is helpful to have a lookup file for
the user to resolve the hashed path.
This is a feature found in vim and commonly used
by Linux kernel test robots to give context about
warnings and/or failures.
e.g. vim +/imem_size +623 drivers/net/ipa/ipa_mem.c
The order in which the commands appear in the args
determines in which order the "goto line:column"
and search will be executed.
Using notrunc will stop the overall truncation of the target file done by sudo.
We need to do this because dd, like other coreutils, already truncates the file
on open(). In case we can't store the backup file afterwards we would end up in
a truncated file for which the user has no write permission by default.
Instead we use a second call of `dd` to perform the necessary truncation
on the command line.
With the fsync option we force the dd process to synchronize the written file
to the underlying device.
When writing a backup file, we should write it atomically (i.e. use a
temporary file + rename) in all cases, not only when replacing an
existing backup. Just like we do in util.SafeWrite(). Otherwise, if
micro crashes while writing this backup, even if that doesn't result
in corrupting an existing good backup, it still results in creating
an undesired backup with invalid contents.
Use the same backup write helper function for both periodic
background backups and for temporary backups in safeWrite().
Besides just removing code duplication, this brings the advantages
of both together:
- Temporary backups in safeWrite() now use the same atomic mechanism
when replacing an already existing backup. So that if micro crashes
in the middle of writing the backup in safeWrite(), this corrupted
backup will not overwrite a previous good backup.
- Better error handling for periodic backups.
When we need to remove existing backup for whatever reason (e.g. because
we've just successfully saved the file), we should do that regardless of
whether backups are enabled or not, since a backup may exist regardless
(it could have been created before the `backup` option got disabled).
We should cancel previously requested periodic backup (and remove this
backup if it has already been created) not only when saving or closing
the buffer but also in other cases when the buffer's state changes from
modified to unmodified, i.e. when the user undoes all unsaved changes.
Otherwise, if micro terminates abnormally before the buffer is closed,
this backup will not get removed (so next time micro will suggest the
user to recover this file), even though all changes to this file were
successfully saved.
Micro's logic for periodic backup creation is racy and may cause
spurious backups of unmodified buffers, at least for the following
reasons:
1. When a buffer is closed, its backup is removed by the main goroutine,
without any synchronization with the backup/save goroutine which
creates periodic backups in the background.
A part of the problem here is that the main goroutine removes the
backup before setting b.fini to true, not after it, so the
backup/save goroutine may start creating a new backup even after it
has been removed by the main goroutine. But even if we move the
b.RemoveBackup() call after setting b.fini, it will not solve the
problem, since the backup/save goroutine may have already started
creating a new periodic backup just before b.fini was set to true.
2. When a buffer is successfully saved and thus its backup is removed,
if there was a periodic backup for this buffer requested by the main
goroutine but not saved by the backup/save goroutine yet (i.e. this
request is still pending in backupRequestChan), micro doesn't cancel
this pending request, so a backup is unexpectedly saved a couple of
seconds after the file itself was saved.
Although usually this erroneous backup is removed later, when the
buffer is closed. But if micro terminates abnormally and the buffer
is not properly closed, this backup is not removed. Also if this
issue occurs in combination with the race issue #1 described above,
this backup may not be successfully removed either.
So, to fix these issues:
1. Do the backup removal in the backup/save goroutine (at requests from
the main goroutine), not directly in the main goroutine.
2. Make the communication between these goroutines fully synchronous:
2a. Instead of using the buffered channel backupRequestChan as a storage
for pending requests for periodic backups, let the backup/save
goroutine itself store this information, in the requestesBackups
map. Then, backupRequestChan can be made non-buffered.
2b. Make saveRequestChan a non-buffered channel as well. (There was no
point in making it buffered in the first place, actually.) Once both
channels are non-buffered, the backup/save goroutine receives both
backup and save requests from the main goroutine in exactly the same
order as the main goroutine sends them, so we can guarantee that
saving the buffer will cancel the previous pending backup request
for this buffer.
Various methods of Buffer should be rather methods of SharedBuffer. This
commit doesn't move all of them to SharedBuffer yet, only those that
need to be moved to SharedBuffer in order to be able to request creating
or removing backups in other SharedBuffer methods.
Instead of calculating the hash of the buffer every time Modified() is
called, do that every time b.isModified is updated (i.e. every time the
buffer is modified) and set b.isModified value accordingly.
This change means that the hash will be recalculated every time the user
types or deletes a character. But that is what already happens anyway,
since inserting or deleting characters triggers redrawing the display,
in particular redrawing the status line, which triggers Modified() in
order to show the up-to-date modified/unmodified status in the status
line. And with this change, we will be able to check this status
more than once during a single "handle event & redraw" cycle, while
still recalculating the hash only once.
- extract the open logic into `openFile()` and return a `wrappedFile`
- extract the closing logic into `Close()` and make a method of `wrappedFile`
- rename `writeFile()` into `Write()` and make a method of `wrappedFile`
This allows to use the split parts alone while keeping overwriteFile() as simple
interface to use all in a row.
Neko Box Coder