GNOME Bugzilla – Bug 786760
Minor refactorings
Last modified: 2017-12-01 12:00:13 UTC
The patches here reduce the amount of intimacy that some classes have with NautilusDirectory, with the exception of the search-directory one - it was just annoying to see it come up in the results for “git grep "directory->details"”. :p
Created attachment 358340 [details] [review] vfs-directory: stop poking internal directory state This commit moves some NautilusVFSDirectory vfunc overrides to NautilusDirectory, as they directly access NautilusDirectoryDetails fields.
Created attachment 358341 [details] [review] directory: reorganize static functions
Created attachment 358342 [details] [review] file: reduce the amount of directory state poking This commit refactors some functions to use the public NautilusDirectory API.
Created attachment 358343 [details] [review] directory: define autoptr cleanup func If any derived classes have one of their own defined, the compiler will start spewing warnings.
Created attachment 358344 [details] [review] search-directory: use GLib type-definition macros
Review of attachment 358340 [details] [review]: Yeah
Review of attachment 358341 [details] [review]: sure
Review of attachment 358342 [details] [review]: nice
Review of attachment 358343 [details] [review]: ah yes
Review of attachment 358344 [details] [review]: Finally! Usually we change to "self" when porting to the new declarations. Can you do that?
(In reply to Carlos Soriano from comment #10) > Review of attachment 358344 [details] [review] [review]: > > Finally! > > Usually we change to "self" when porting to the new declarations. Can you do > that? I’ve actually never heard of that, but sure, for consistency.
Created attachment 358353 [details] [review] search-directory: rename method instance parameters The new standard around here is “self”.
Review of attachment 358353 [details] [review]: Looks good now! Just as a small comment, usually "self" is not used for the public API, but for the implementation. I like it in the public API as you did though, and usually I do it that way as well.
Attachment 358340 [details] pushed as c02a1f4 - vfs-directory: stop poking internal directory state Attachment 358341 [details] pushed as 98f3b91 - directory: reorganize static functions Attachment 358342 [details] pushed as 97cb796 - file: reduce the amount of directory state poking Attachment 358343 [details] pushed as 2f76227 - directory: define autoptr cleanup func Attachment 358344 [details] pushed as aff4e92 - search-directory: use GLib type-definition macros Attachment 358353 [details] pushed as 17a4f43 - search-directory: rename method instance parameters
Created attachment 358401 [details] [review] file: add private method to get directory This is to be used instead of accessing the directory directly.
Created attachment 358402 [details] [review] directory: get file directories from new API Another attempt at reducing the amount of internal state accessing.
Created attachment 358403 [details] [review] directory: use public API to get name of the file This additionally removes some redundant assertions to avoid needless string copies.
Some more patches here.
Created attachment 358412 [details] [review] search-directory-file: access file directory using new API
Created attachment 358413 [details] [review] file: add API to get recency and trash time
Created attachment 358414 [details] [review] vfs-file: use API to get trash time and recency
Review of attachment 358401 [details] [review]: Sure, just a nitpick for future: this is a case where splitting patches doesn't make much sense, because there is no use of this API.
Review of attachment 358402 [details] [review]: LGTM!
Review of attachment 358403 [details] [review]: +1
Review of attachment 358412 [details] [review]: yes!
Review of attachment 358413 [details] [review]: I'm starting to get you now...
Review of attachment 358414 [details] [review]: +1
Created attachment 358421 [details] [review] vfs-file: use API to get file directory object
Created attachment 358422 [details] [review] vfs-file: use API to get file type
Created attachment 358423 [details] [review] vfs-file: move some vfunc overrides to NautilusFile Some method overrides are only accessing the private data of the parent class, so those might as well live in the parent class.
Review of attachment 358421 [details] [review]: It's good, except that the ownership in the gets shouldn't be duplicated.
Review of attachment 358422 [details] [review]: +1
Created attachment 358424 [details] [review] directory: use API to get file directory objects Another attempt at reducing the amount of internal state accessing.
Created attachment 358425 [details] [review] directory: use API to get file names This additionally removes some redundant assertions to avoid needless string copies.
Created attachment 358426 [details] [review] search-directory-file: use API to access file directory objects
Created attachment 358427 [details] [review] vfs-file: use API to get file times Additionally this adds new API for trash times and recencies.
Created attachment 358428 [details] [review] vfs-file: use API to get file directory objects
Created attachment 358429 [details] [review] vfs-file: use API to get file type
Created attachment 358430 [details] [review] vfs-file: move some vfunc overrides to NautilusFile Some method overrides are only accessing the private data of the parent class, so those might as well live in the parent class.
Attachment 358424 [details] pushed as 364a359 - directory: use API to get file directory objects Attachment 358425 [details] pushed as d56e570 - directory: use API to get file names Attachment 358426 [details] pushed as 7ba0a40 - search-directory-file: use API to access file directory objects Attachment 358427 [details] pushed as 3aab6e9 - vfs-file: use API to get file times Attachment 358428 [details] pushed as 1cea755 - vfs-file: use API to get file directory objects Attachment 358429 [details] pushed as 2470d82 - vfs-file: use API to get file type Attachment 358430 [details] pushed as cbcbb3a - vfs-file: move some vfunc overrides to NautilusFile