-
Notifications
You must be signed in to change notification settings - Fork 137
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
Improve source management Control #617
Conversation
Further ideas:
|
Not according to QFileDialog's enums. It's either files or a folder. |
He's right. I already tried that. Can't mix files and folders unless there is a custom file selector class (maybe worth it). |
Found this for files and folders: Thiis for multiple directories: |
I think it's great if it works without causing lots of background disk activity. And the calculation needs to happen async in the background. You'll need a placeholder before it's done. If people add network drives it could be even slower. Also suggest to add an option under Misc to disable it. |
Not UI related, but: |
It doesn't cause disk activity in the background (Well, it does once, when it calculates the sizie initial). An additional button to manually update the sizes could be placed somewhere... |
Yeah. That will be the issue. If this runs whenever the profile changes, it will cause heavy disk activity. |
Hm, are you sure you want to go the unix way? I sticked to QT for portability (Future windows version, for example), but I'm up for discussion. Also, currently there's an artificial delay for 5 seconds (for testing) before each thread returns. |
I would only calculate when adding or when it's manually refreshed. Whenever it's opened will be too often. E.g. I have multiple profiles to do different home folder backups. So it would size my whole home folder multiple times? Also: how about Borg exclusions below? Once I exclude large folders, the calculated total will be meaningless. :-( This is a hard problem. |
Maybe calculate the exclusion size separately and subtract? Would lead to double the IO worst case. |
I didn't think about exclusions for now tbh, since the idea was to only show the current size of each entry, but true, that's an issue aswell. If you don't want the size to be calculated on startup, it needs to be saved in the database, along with the path I guess? |
Yeah, should be part of SourceFileModel |
Yes. Good idea. We don't have much in that table yet. At least it will be used for something. So I'd only calculate the size when adding it or manually refreshing. Together with a checkbox in Misc to disable calculating it when adding. E.g. "Calculate source folder statistics when adding." Enabled by default. |
Okay, I'll do some reworks. BTW what's the best practice in python for interfacing with native APIs (the mentioned |
@XXXBold Marginal observation, which I would like to share when I look at the screenshot above: |
If a lot of work is done by the CLI tool and the CLI tool has no mode for json (or similar) structured output, then calling it from E.g. I used to size folders in Python, but also use
macOS is most likely just |
Not working yet, requires a new DB version I guess. Would appreciate any help in this regard
I've added new attributes for the SourceFileModel. However, I think this requires a new DB-Version? Would appreciate if someone could help me with this, no idea what to do there :P Also, if this changes anyway, should the "dir" attribute be renamed to "dirPath"? Would just be more obvious then. |
You need to add schema upgrading code in init_db, for this:
Along with bumping |
Thanks! I've also considered your hint for the naming convention, and changed the variable names, will also look over the other files... |
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Thanks for the contribution, @XXXBold . Great to see everyone tackling a problem to arrive at the best solution. Let's proceed to review and then merge. |
Just tried this locally. Works well and not intrusive, if someone doesn't want/need to calculate the sizes. Some thoughts I had:
And for future improvements:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality is fine, just some minor style changes.
For the alternating colours, IMO it looks cleaner with the same colour throughout. |
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Co-authored-by: Samuel <samuel.woon@protonmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I think about it, Decimal is not really needed.
Co-authored-by: Samuel <samuel.woon@protonmail.com>
Co-authored-by: Samuel <samuel.woon@protonmail.com>
What's the status here? Are we good to merge? Since this is a larger PR, would be good to get it merged before we get conflicts with other PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Tested:
- Empty folder (Size 0B File Count 0)
- Folder with one file (Size 2B File Count 1)
Might want to add a note on the sources tab about the size reducing with compression.
Later we could use the exclude patterns and exclude if present to get a more accurate total size.
Works for me as well. Thanks for the great contribution, @XXXBold !
Let's save those for later, when we re-arrange the whole Source tab to use multiple toolbox areas (source folders, exclusions). The PR is already large enough. |
This PR aims to change the source tab to show more information, as suggested in #593 no. 4.
I think some discussion is required in regards of design/implementation details
State:
PR is feature complete, some issue on Mac OS remaining...
First Preview of new source UI: