Re: [PATCH 03/13] list-objects: filter objects in traverse_commit_list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/25/2017 12:05 AM, Jonathan Tan wrote:
On Tue, Oct 24, 2017 at 11:53 AM, Jeff Hostetler <git@xxxxxxxxxxxxxxxxx> wrote:

+enum list_objects_filter_result {
+       LOFR_ZERO      = 0,
+       LOFR_MARK_SEEN = 1<<0,

Probably worth documenting, something like /* Mark this object so that
it is skipped for the rest of the traversal. */

+       LOFR_SHOW      = 1<<1,

And something like /* Invoke show_object_fn on this object. This
object may be revisited unless LOFR_MARK_SEEN is also set. */

+};
+
+/* See object.h and revision.h */
+#define FILTER_REVISIT (1<<25)

I think this should be declared closer to its use - in the sparse
filter code or in the file that uses it. Wherever it is, also update
the chart in object.h to indicate that we're using this 25th bit.

+
+enum list_objects_filter_type {
+       LOFT_BEGIN_TREE,
+       LOFT_END_TREE,
+       LOFT_BLOB
+};
+
+typedef enum list_objects_filter_result list_objects_filter_result;
+typedef enum list_objects_filter_type list_objects_filter_type;

I don't think we typedef enums in Git code.

+
+typedef list_objects_filter_result (*filter_object_fn)(
+       list_objects_filter_type filter_type,
+       struct object *obj,
+       const char *pathname,
+       const char *filename,
+       void *filter_data);
+
+void traverse_commit_list_worker(
+       struct rev_info *,
+       show_commit_fn, show_object_fn, void *show_data,
+       filter_object_fn filter, void *filter_data);

I think things would be much clearer if a default filter was declared
(matching the behavior as of this patch when filter == NULL), say:
static inline default_filter(args) { switch(filter_type) { case
LOFT_BEGIN_TREE: return LOFR_MARK_SEEN | LOFR_SHOW; case
LOFT_END_TREE: return LOFT_ZERO; ...

And inline traverse_commit_list() instead of putting it in the .c file.

This would reduce or eliminate the need to document
traverse_commit_list_worker, including what happens if filter is NULL,
and explain how a user would make their own filter_object_fn.

+
+#endif /* LIST_OBJECTS_H */
--
2.9.3


I'll give that a try.  Thanks!

Jeff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux