Re: [PATCH1/2] Libify blame
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 18, 2009, 05:41 UTC
- Message-ID
- <7vocvzmlqf.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <49BE5343.60900@gmail.com>
pi song <pi.songs@gmail.com> writes:
Show 15 quoted lines
> diff --git a/blame.h b/blame.h
> new file mode 100644
> index 0000000..72d1e2a
> --- /dev/null
> +++ b/blame.h
> @@ -0,0 +1,166 @@
> +/*
> + * for storing stats. it can be used
> + * across multiple blame operations
> + */
> +struct blame_stat {
> + int num_read_blob;
> + int num_get_patch;
> + int num_commits;
> +};As I said in my previous message, I do not understand why this is not part of the super-scoreboard (now blame_info).
Show 11 quoted lines
> +#define PICKAXE_BLAME_MOVE 01 > +#define PICKAXE_BLAME_COPY 02 > +#define PICKAXE_BLAME_COPY_HARDER 04 > +#define PICKAXE_BLAME_COPY_HARDEST 010 > + > +#define BLAME_DEFAULT_MOVE_SCORE 20 > +#define BLAME_DEFAULT_COPY_SCORE 40 > + > +/* bits #0..7 in revision.h, #8..11 used for merge_bases() in commit.c */ > +#define METAINFO_SHOWN (1u<<12) > +#define MORE_THAN_ONE_PATH (1u<<13)
Do we need to expose all of these constants outside blame.c? I think the library caller needs access to the first four above, but I tend to think the latter four are purely internal implementation detail that should be kept in blame.c.
Show 9 quoted lines
> +/* output formatting constants */ > +#define OUTPUT_ANNOTATE_COMPAT 001 > +#define OUTPUT_LONG_OBJECT_NAME 002 > +#define OUTPUT_RAW_TIMESTAMP 004 > +#define OUTPUT_PORCELAIN 010 > +#define OUTPUT_SHOW_NAME 020 > +#define OUTPUT_SHOW_NUMBER 040 > +#define OUTPUT_SHOW_SCORE 0100 > +#define OUTPUT_NO_AUTHOR 0200
I think these can be public.
Show 11 quoted lines
> +/*
> + * One blob in a commit that is being suspected
> + */
> +struct origin {
> + int refcnt;
> + struct origin *previous;
> + struct commit *commit;
> + mmfile_t file;
> + unsigned char blob_sha1[20];
> + char path[FLEX_ARRAY];
> +};I somehow doubt we would want to expose this level of implementation detail to the callers of the library. If we need to, the structure needs to be renamed---"origin" is way too generic a name.
> +extern void assign_blame(struct blame_scoreboard *sb, int opt) ;
Lose the extra SP before ";". I had to fix them in your previous patch and there were many.