Symlink size in File Properties dialog

[tsujan]

There's a discrepancy between what the File Properties dialog shows and what can be seen on the statusbar in the case of symlinks.

Expected Behavior

File Properties dialog should show the target size, as the statusbar shows.

Current Behavior

File Properties dialog shows the real symlink size, for example 8 bytes (8 B), which isn't informative.

System Information

Latest git.

EDIT: It's better to add "Target size:" in addition to "File size:".

[tsujan]

On second thought, can we see this as a feature? 2 reports with different meanings?

[mbwk]

Hi, I decided to take a look at this issue as a fun introduction to the code.

After some reading and figuring things out, I found that the relevant code for this exists in libfm-qt under filepropsdialog.cpp. Specifically this usage of TotalSizeJob.

The quick fix would be to change the flags from Fm::TotalSizeJob::DEFAULT to Fm::TotalSizeJob::FOLLOW_LINKS. From a user standpoint though, I see the rationale for having "Target size:" in addition to "File size:", and this quick fix would not allow for that.

(I was unsure whether to post a reply here, or to create a duplicate issue in libfm-qt. As I looked into this more, I could see how this could be split off into its own separate feature to change the functionality of TotalSizeJob)

Basic Solution: Add a second total size job

Check if file list contains symlinks, and use a second TotalSizeJob with the follow_links flag to sum the target sizes, which can then be displayed in the dialog.

totalSizeJob = new Fm::TotalSizeJob(fileInfos_.paths(), Fm::TotalSizeJob::DEFAULT);
targetSizeJob = new Fm::TotalSizeJob(fileInfos_.paths(), Fm::TotalSizeJob::FOLLOW_LINKS);

Simple, but inefficient. It doubles the run time of summing file sizes for filepropsdialog, pointlessly recounting potentially many files that aren't symlinks. Acceptable for single files, but not great for a selection of files (which seems to be the main point for the design of TotalSizeJob).

Messy Solution: Revise TotalSizeJob to hold both File and Target sizes

A messier fix. Add a second set of member variables alongside the existing totalSize_ and totalOndiskSize_ that can be used to store target size, and either add another flag or change the behaviour of FOLLOW_LINKS to count both the "real" file size and target file size in a single pass here.

More efficient, but arguably messier code. Particularly because it would require duplicating things like variables (to hold both sizes), queries (to query both files) and add more branching to the code.

Conclusion

I have other vague ideas floating around that I'm not quite ready to put into words. Some might require potentially major changes to TotalSizeJob. Maybe replacing it altogether with a generic file querying object.

Either way, I felt it would probably be wise to consult maintainers before messing up their code. There's probably a more elegant solution I'm missing.

[tsujan]

Please read above ("On second thought,..."). I'm not sure if it's good to change it.

[tsujan]

If other devs/users think that it's a good idea, it could be done by changing TotalSizeJob, so that it also gets FileInfoList as an argument and follows symlinks when a file isn't a folder. That's basically how Nautilus does it: it ignores symlink targets with folders but considers them with files. The logic is as simple as that.

I made a patch last year but didn't find the main idea interesting, especially because no one added a comment.