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

added item_count #1492

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

added item_count #1492

wants to merge 4 commits into from

Conversation

Rishab87
Copy link

Fixes #1491, This PR adds item_count

Copy link
Collaborator

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

This is looking good to me!

I think two things need to be added here:

I think this is fully backwards compatible but wanted to run it by @Zsailer to confirm.

@Rishab87 Rishab87 marked this pull request as ready for review January 24, 2025 16:47
@Rishab87
Copy link
Author

@krassowski , I've changed tests and doc and ensured no tests are failing, ig its ready to be reviewed now

@krassowski
Copy link
Collaborator

does length of item_count should be equal to content's length? as content excludes some of the files

Very good point! I just checked in two file managers (Nautilus and Nemo) and both do show the "1 item" if a folder includes both a hidden file and a non-hidden file by default, but if user selects "show hidden files" then they show "2 items".

I wonder if there is a potential performance consideration here though..

@Rishab87
Copy link
Author

Rishab87 commented Jan 24, 2025

For now I've added the filters to not count hidden files or any other file which are not visible on jupyter lab's file browser by default (which means item_count and len(contents) are equal),

But yeah I otherwise need to somehow know if a user has selected show hidden folders in settings if yes then count those files too,

not sure if it will cause any performance issues need to look into the code in more detail! Also noticed one more thing even after turning on show hidden folders in jupyter labs I was not able to see all files which were otherwise easily visible in files app, so I think it might not be necessary to add count of hidden folders or files especially if it may cause performance issues and can lead to inconsistent item counts and actual items visible though I can be completely wrong here

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

Successfully merging this pull request may close these issues.

Return no of items inside a directory for filebrowser
2 participants