Skip to content
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.

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

[Bug]: If a folder is soft link, it will be treated as a file and not a folder #1694

Open
3 tasks
abgox opened this issue Jan 8, 2025 · 9 comments
Open
3 tasks
Labels
bug Something isn't working triage Needs triage

Comments

@abgox
Copy link

abgox commented Jan 8, 2025

Current Behavior

  • If a folder is soft link, it will be treated as a file and not a folder. And associated icons, types, etc. are all wrong.

recording

Expected Behavior

They should be correctly recognized as a folder.

Steps To Reproduce

When you use File Preview to browse a file directory, this error occurs whenever a folder in the directory is soft link.

Wave Version

0.10.4

Platform

Windows

OS Version/Distribution

Windows 11

Architecture

x64

Anything else?

No response

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later
@abgox abgox added bug Something isn't working triage Needs triage labels Jan 8, 2025
@Jalileh
Copy link
Contributor

Jalileh commented Jan 9, 2025

hey are you using cmd's mklink to create symbolic links by any chance?

I reproduced it with 3 shells and only cmd caused the bug
image

@abgox
Copy link
Author

abgox commented Jan 10, 2025

  • I use the pwsh command New-Item -ItemType SymbolicLink xxx to create the soft link.
  • I think pwsh and cmd may work the same way.

@Jalileh
Copy link
Contributor

Jalileh commented Jan 10, 2025

  • I use the pwsh command New-Item -ItemType SymbolicLink xxx to create the soft link.
  • I think pwsh and cmd may work the same way.

Noticed more of my folders were also bugged and I don't use CMD.

Doesn't matter if we dont use cmd, other scripts/programs will eventually use it like the .config folder.

Golang os package literally wont return the bits for CMD made symlinks from fileInfo.Mode all it returns is read perm bits.

EDIT: nevermind, fixed it.

Jalileh added a commit to Jalileh/waveterm that referenced this issue Jan 10, 2025
windows symlinks are strange and they are not treated as directories in
go, so using just fileInfo.Dir isn't enough.
@esimkowitz
Copy link
Member

closing as we're about to release 0.11. Thank you @Jalileh for fixing this!!

@abgox
Copy link
Author

abgox commented Jan 27, 2025

@esimkowitz
Copy link
Member

esimkowitz commented Jan 27, 2025

@abgox what utility do you use to make your symlinks? Could you provide reproduction steps?

@Jalileh
Copy link
Contributor

Jalileh commented Jan 27, 2025

@esimkowitz

Yeah sorry, the bug here is in the dot of the symlink folder name ( .config )
PR bot ironically made it less robust at the cost of my over-engineered approach

 	isFileSymlink := func(filepath string) bool {
		Length := len(filepath) - 1
		maxFileDotExt := 4 // should cover most file extensions
		for i := Length; i >= (Length - maxFileDotExt); i-- {
			if filepath[i] == '.' {
				return true
			}
		}
		return false
	}

Image

isFileSymlink := func(filepath string) bool {
+        if len(filepath) == 0 {
+            return false
+        }
+        return strings.LastIndex(filepath, ".") > strings.LastIndex(filepath, "/")
 }

Image

@abgox
Copy link
Author

abgox commented Jan 28, 2025

@abgox what utility do you use to make your symlinks? Could you provide reproduction steps?

  • I use the pwsh command New-Item -ItemType SymbolicLink xxx to create the soft link.

Yeah sorry, the bug here is in the dot of the symlink folder name ( .config ) PR bot ironically made it less robust at the cost of my over-engineered approach

  • That's right, the bug only exists in symlink folders with .

  • As long as the directory name contains . , the bug exists.

    Image

@esimkowitz esimkowitz reopened this Jan 28, 2025
@Jalileh
Copy link
Contributor

Jalileh commented Jan 28, 2025

fixing this right now and realize the AI refractor doesnt even improve upon my method,
windows doesnt even use / for directories it uses \\

this line literally does nothing

return strings.LastIndex(filepath, ".") > strings.LastIndex(filepath, "/"),

I'm going to open a PR and revert back to the one I wrote before and hopefully make it less linear and unreliable so AI dosen't suggest anything worse again. Soz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Needs triage
Projects
None yet
Development

No branches or pull requests

3 participants