Skip to content

buffer: Store potential resolved symlinks as AbsPath #3997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JoeKar wants to merge 3 commits into
base: master
Choose a base branch
from
+48 −12
Open

buffer: Store potential resolved symlinks as AbsPath #3997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8fbfb1
buffer: Resolve symlinks in file paths
JoeKar Feb 11, 2026
f3ea704
buffer: Don't cancel the backup in case the buffer is shared
JoeKar Feb 5, 2026
ddda4d0
buffer: Don't `Serialize` in case the buffer is shared
JoeKar Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cmd/micro/micro_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/micro-editor/micro/v2/internal/buffer"
"github.com/micro-editor/micro/v2/internal/config"
"github.com/micro-editor/micro/v2/internal/screen"
"github.com/micro-editor/micro/v2/internal/util"
"github.com/micro-editor/tcell/v2"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -157,8 +158,9 @@ func openFile(file string) {

func findBuffer(file string) *buffer.Buffer {
var buf *buffer.Buffer
_, file = util.ResolvePath(file)
for _, b := range buffer.OpenBuffers {
if b.Path == file {
if b.AbsPath == file {
buf = b
}
}
Expand Down
17 changes: 10 additions & 7 deletions internal/buffer/buffer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,9 @@ func NewBufferFromString(text, path string, btype BufType) *Buffer {
// Places the cursor at startcursor. If startcursor is -1, -1 places the
// cursor at an autodetected location (based on savecursor or :LINE:COL)
func NewBuffer(r io.Reader, size int64, path string, btype BufType, cmd Command) *Buffer {
absPath, err := filepath.Abs(path)
if err != nil {
absPath = path
absPath := path
if btype == BTDefault && path != "" {
path, absPath = util.ResolvePath(path)
}

b := new(Buffer)
Expand Down Expand Up @@ -391,6 +391,7 @@ func NewBuffer(r io.Reader, size int64, path string, btype BufType, cmd Command)
}
config.UpdatePathGlobLocals(b.Settings, absPath)

var err error
b.encoding, err = htmlindex.Get(b.Settings["encoding"].(string))
if err != nil {
b.encoding = unicode.UTF8
Expand Down Expand Up @@ -489,7 +490,7 @@ func NewBuffer(r io.Reader, size int64, path string, btype BufType, cmd Command)
}
}

err = config.RunPluginFn("onBufferOpen", luar.New(ulua.L, b))
err := config.RunPluginFn("onBufferOpen", luar.New(ulua.L, b))
if err != nil {
screen.TermMessage(err)
}
Expand Down Expand Up @@ -524,10 +525,12 @@ func (b *Buffer) Close() {
// Fini should be called when a buffer is closed and performs
// some cleanup
func (b *Buffer) Fini() {
if !b.Modified() {
b.Serialize()
if !b.Shared() {
if !b.Modified() {
b.Serialize()
}
b.CancelBackup()
}
b.CancelBackup()

if b.Type == BTStdout {
fmt.Fprint(util.Stdout, string(b.Bytes()))
Expand Down
5 changes: 1 addition & 4 deletions internal/buffer/save.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,7 @@ func (b *Buffer) saveToFile(filename string, withSudo bool, autoSave bool) error
return errors.New("Error: " + filename + " is not a regular file and cannot be saved")
}

absFilename, err := filepath.Abs(filename)
if err != nil {
return err
}
filename, absFilename := util.ResolvePath(filename)

// Get the leading path to the file | "." is returned if there's no leading path provided
if dirname := filepath.Dir(absFilename); dirname != "." {
Expand Down
34 changes: 34 additions & 0 deletions internal/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,40 @@ func DetermineEscapePath(dir string, path string) (string, string) {
return url, ""
}

// ResolvePath provides the absolute file path for the given relative file path
// as well as resolves symlinks in both. It returns the relative path with
// symlinks resolved and the absolute path with symlinks resolved. If it fails
// to get the absolute path or to resolve symlinks, it returns unresolved path
// in place of resolved one. The only exception is the case in which the target
// file doesn't exist. We leave the path handling to EvalSymlinks() and use the
// path causing the error as target path.
Copy link
Copy Markdown
Collaborator

@dmaluka dmaluka Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only exception is the case in which the target file doesn't exist. We leave the path handling to EvalSymlinks() and use the path causing the error as target path.

This sounds too detailed, and at the same time completely unclear. What "path handling", etc.

Why not just e.g. "In case the file does not exist, ResolvePath still resolves it, by resolving its directory path and keeping the file name." ?

func ResolvePath(path string) (string, string) {
resolvedPath, err := filepath.EvalSymlinks(path)
if err == nil {
path = resolvedPath
} else if errors.Is(err, fs.ErrNotExist) {
if e, ok := err.(*fs.PathError); ok {
path = e.Path
}
}

absPath, err := filepath.Abs(path)
if err != nil {
absPath = path
}

resolvedPath, err = filepath.EvalSymlinks(absPath)
Copy link
Copy Markdown
Collaborator

@dmaluka dmaluka Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed another corner-case issue: if the file doesn't exist yet (so it is opened as an empty file) and its directory path contains symlinks, those symlinks are not resolved. E.g.:

mkdir dir
ln -s dir dir1
micro -multiopen vsplit dir/file dir1/file

For comparison, again, vim correctly resolves it (thus correctly detects that dir/file and dir/file1 are the same file, even though this file doesn't exist yet).

Copy link
Copy Markdown
Member Author

@JoeKar JoeKar Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good catch! 👍
Caused by symlink.go;l=84-87 and stat_unix.go;l=47-49, but fortunately it is wrapped into a PathError containing the causing path name. Here we can apply a little "hack" where EvalSymlinks() still does the job with the path and we pick it from the error message.

Copy link
Copy Markdown
Collaborator

@dmaluka dmaluka Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we relying on some undocumented behavior here?

There is another corner case: the file doesn't exist, and its directory doesn't exist either, but some of its ancestor directories exists and contains symlinks:

mkdir dir
ln -s dir dir1
micro dir1/dir/file

As you can see, this newest version handles this case utterly incorrectly.

Furthermore, even if there are no symlinks (e.g. micro dir/dir/file with the above example), it behaves in the same utterly incorrect way. I.e. it causes a regression.

Copy link
Copy Markdown
Member Author

@JoeKar JoeKar Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, EvalSymlinks() stops at the first unavailable dir or file inside the path.
Unfortunately, it's not that simple anymore. 🤔

Copy link
Copy Markdown
Member Author

@JoeKar JoeKar Mar 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we should use a similar approach as vim does. We iterative resolve path element per path element till we encounter an issue, but only in case EvalSymlinks() returns ErrNotExist on first try.

First tests seem promising and micro fails, where vim fails too:

mkdir dir1 dir3
ln -s dir1 dir2
ln -s ../dir1/file dir3/file
micro dir1/file dir2/file dir3/file
vim -p dir1/file dir2/file dir3/file

cd dir2/
micro ../dir1/file file ../dir3/file
vim -p ../dir1/file file ../dir3/file
cd ..

micro dir/dir/file
micro dir1/dir/file

dir3/file can't be resolved by both to the same file dir1/file. The rest was working.

PS:

Are we relying on some undocumented behavior here?

Yes, it was a bad hack 😓

if err == nil {
absPath = resolvedPath
} else if errors.Is(err, fs.ErrNotExist) {
if e, ok := err.(*fs.PathError); ok {
absPath = e.Path
}
}

return path, absPath
}

// GetLeadingWhitespace returns the leading whitespace of the given byte array
func GetLeadingWhitespace(b []byte) []byte {
ws := []byte{}
Expand Down
Loading