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

Improving file panel rendering. #589

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

lazysegtree
Copy link
Collaborator

@lazysegtree lazysegtree commented Feb 4, 2025

Removed redundant computations, and improved error handling.
Fixes some issue found during investigation of #585
Renamed folder to dir

Validated that file view and sort options were working as intended.

Validated that no issues in opening root directory \
Validated that hiding and showing of dot files was working as expected

Copy link

netlify bot commented Feb 4, 2025

Deploy Preview for superfile canceled.

Name Link
🔨 Latest commit c3d9fea
🔍 Latest deploy log https://app.netlify.com/sites/superfile/deploys/67a25da14669c1000818f8ee

@lazysegtree lazysegtree force-pushed the fix_tmp_issue_585 branch 2 times, most recently from bc68644 to da3c86e Compare February 4, 2025 18:20
sort.Slice(files, order)

for _, item := range files {
fileInfo, err := item.Info()
Copy link
Collaborator Author

@lazysegtree lazysegtree Feb 4, 2025

Choose a reason for hiding this comment

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

No need of filtering dirEntries after. Filtered them before the sort.

name: item.Name(),
directory: item.IsDir(),
}
if location == "/" {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is redundant. filepath.Join() handles location being "/"

switch sortOptions.options[sortOptions.selected] {
case "Name":
order = func(i, j int) bool {
_, errI := os.ReadDir(location + "/" + files[i].Name())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We dont need to do ReadDir syscall here at all. We can just decide based on IsDir()

@lazysegtree lazysegtree marked this pull request as ready for review February 4, 2025 18:25
@lazysegtree lazysegtree requested a review from yorukot February 4, 2025 18:32
@lazysegtree
Copy link
Collaborator Author

Rebased to latest main, after merge of PR#584

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant