Libify blame
The patch does extract blame.h
--- Makefile | 1 + blame.h | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ builtin-blame.c | 145 +----------------------------------------------- 3 files changed, 169 insertions(+), 143 deletions(-) create mode 100644 blame.h
Makefile +1 -0
@@ -359,6 +359,7 @@ XDIFF_LIB=xdiff/lib.a LIB_H += archive.h LIB_H += attr.h +LIB_H += blame.h LIB_H += blob.h LIB_H += builtin.h LIB_H += cache.h
blame.h new file +166 -0
166 lines not on this page · raw message
builtin-blame.c +2 -143
169 lines not on this page · raw message
Signature · 1 line
1.5.4.3
Quoted 6 lines · pi song
blame.h new file +0
-0
blame.h line 9
Quoted 9 lines
blame.h new file +9
-0@@ -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).
blame.h line 32
Quoted 11 lines
blame.h new file +11
-0+#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.
blame.h line 42
Quoted 9 lines
blame.h new file +9
-0+/* 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.
blame.h line 54
Quoted 11 lines
blame.h new file +11
-0+/* +* 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.
blame.h line 164
Quoted 1 line
blame.h new file +1
-0+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.
Don't you think we should rather split up into smaller files before start reorganizing things?
Pi Song
Quoted 71 lines · Junio C Hamano
Quoted 6 lines · pi song
blame.h new file +0
-0blame.h line 9
Quoted 9 lines
blame.h new file +9
-0@@ -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).blame.h line 32
Quoted 11 lines
blame.h new file +11
-0+#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 inblame.c.blame.h line 42
Quoted 9 lines
blame.h new file +9
-0+/* 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 0200I think these can be public.
blame.h line 54
Quoted 11 lines
blame.h new file +11
-0+/* +* 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.
blame.h line 164
Quoted 1 line
blame.h new file +1
-0+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.
Quoted 2 lines · pi song
Don't you think we should rather split up into smaller files before start reorganizing things?
Yes, but splitting it wrong is, eh, wrong ;-)
Wait. If you look at the builtin-blame.c, out of question it is very messy. Things like print_usage() or -L parameter parsing for example is not done upfront but hiding somewhere. Some functions are not very clear if they are frontend or backend. I would say nobody would be able to split it right in the first place. What you could do is to split it to something "roughly right" and then work from that.
My latest two patches really do nothing but just splitting files. I haven't changed any logics or renamed any thing only to make this big beast more *manageable* rather than tackling the problem directly. Yes, some bits are still wrong but I believe 70% of the functions should already stay in the right place. The following patches will make the structure more right *gradually*.
Pi Song
Quoted 7 lines · Junio C Hamano
Quoted 2 lines · pi song
Don't you think we should rather split up into smaller files before start reorganizing things?
Yes, but splitting it wrong is, eh, wrong ;-)
BTW, following patches are not available yet : P
Quoted 26 lines · pi song
Wait. If you look at the
builtin-blame.c, out of question it is very messy. Things likeprint_usage()or -L parameter parsing for example is not done upfront but hiding somewhere. Some functions are not very clear if they are frontend or backend. I would say nobody would be able to split it right in the first place. What you could do is to split it to something "roughly right" and then work from that.My latest two patches really do nothing but just splitting files. I haven't changed any logics or renamed any thing only to make this big beast more *manageable* rather than tackling the problem directly. Yes, some bits are still wrong but I believe 70% of the functions should already stay in the right place. The following patches will make the structure more right *gradually*.
Pi Song
Quoted 7 lines · Junio C Hamano
Quoted 2 lines · pi song
Don't you think we should rather split up into smaller files before start reorganizing things?
Yes, but splitting it wrong is, eh, wrong ;-)