Re: [PATCH mh/lockfile-retry] lockfile: replace random() by rand()
Johannes Sixt j...@kdbg.org writes:
There you have it: Look the other way for a while, and people start
using exotic stuff... ;)
Is it exotic to have random/srandom? Both are in POSIX and 4BSD;
admittedly rand/srand are written down in C89 and later, so they
might be more portable, but I recall the prevailing wisdom is to
favor random over rand for quality of randomness and portability, so
I am wondering if it may be a better approach to keep the code as-is
and do a compat/random.c based on either rand/srand (or use posix
sample implementation [*1*]).
[Reference]
*1* http://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html
This is a build breakage of master on Windows. There are also a few
new test suite failures. On of them is in t1404#2, indicating that
a DF conflict takes a different error path. I haven't debugged, yet.
The lock file retry test fails, too. I'll report back as time permits.
lockfile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index 5a93bc7..ee5cb01 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -191,7 +191,7 @@ static int lock_file_timeout(struct lock_file *lk, const
char *path,
return lock_file(lk, path, flags);
if (!random_initialized) {
- srandom((unsigned int)getpid());
+ srand((unsigned int)getpid());
random_initialized = 1;
}
@@ -218,7 +218,7 @@ static int lock_file_timeout(struct lock_file *lk, const
char *path,
backoff_ms = multiplier * INITIAL_BACKOFF_MS;
/* back off for between 0.75*backoff_ms and 1.25*backoff_ms */
- wait_us = (750 + random() % 500) * backoff_ms;
+ wait_us = (750 + rand() % 500) * backoff_ms;
sleep_microseconds(wait_us);
remaining_us -= wait_us;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[WIP/PATCH v4 4/8] for-each-ref: introduce new structures for better organisation
Intoduce 'ref_filter_cbdata' which will hold 'ref_filter'
(Conditions to filter the refs on) and 'ref_array' (The array
of ref_array_items). Modify the code to use these new structures.
This is a preparatory patch to eventually move code from 'for-each-ref'
to 'ref-filter' and making it publically available.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 56 --
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e634fd2..ef54c90 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -39,6 +39,20 @@ struct ref_array_item {
char *refname;
};
+struct ref_array {
+ int nr, alloc;
+ struct ref_array_item **items;
+};
+
+struct ref_filter {
+ const char **name_patterns;
+};
+
+struct ref_filter_cbdata {
+ struct ref_array array;
+ struct ref_filter filter;
+};
+
static struct {
const char *name;
cmp_type cmp_type;
@@ -85,7 +99,7 @@ static struct {
* a * to denote deref_tag().
*
* We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects. ref_array_item
+ * of properties that we need to extract out of objects. ref_array_item
* structure will hold an array of values extracted that can be
* indexed with the atom number, which is an index into this
* array.
@@ -830,12 +844,6 @@ static void get_value(struct ref_array_item *ref, int
atom, struct atom_value **
*v = ref-value[atom];
}
-struct grab_ref_cbdata {
- struct ref_array_item **grab_array;
- const char **grab_pattern;
- int grab_cnt;
-};
-
/*
* Given a refname, return 1 if the refname matches with one of the patterns
* while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
@@ -879,7 +887,8 @@ static struct ref_array_item *new_ref_array_item(const char
*refname,
*/
static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
- struct grab_ref_cbdata *cb = cb_data;
+ struct ref_filter_cbdata *ref_cbdata = cb_data;
+ struct ref_filter *filter = ref_cbdata-filter;
struct ref_array_item *ref;
if (flag REF_BAD_NAME) {
@@ -887,13 +896,13 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
return 0;
}
- if (*cb-grab_pattern !match_name_as_path(cb-grab_pattern, refname))
+ if (*filter-name_patterns
!match_name_as_path(filter-name_patterns, refname))
return 0;
ref = new_ref_array_item(refname, sha1, flag);
- REALLOC_ARRAY(cb-grab_array, cb-grab_cnt + 1);
- cb-grab_array[cb-grab_cnt++] = ref;
+ REALLOC_ARRAY(ref_cbdata-array.items, ref_cbdata-array.nr + 1);
+ ref_cbdata-array.items[ref_cbdata-array.nr++] = ref;
return 0;
}
@@ -937,10 +946,10 @@ static int compare_refs(const void *a_, const void *b_)
return 0;
}
-static void sort_refs(struct ref_sort *sort, struct ref_array_item **refs, int
num_refs)
+static void sort_refs(struct ref_sort *sort, struct ref_array *array)
{
ref_sort = sort;
- qsort(refs, num_refs, sizeof(struct ref_array_item *), compare_refs);
+ qsort(array-items, array-nr, sizeof(struct ref_array_item *),
compare_refs);
}
static void print_value(struct atom_value *v, int quote_style)
@@ -1076,12 +1085,12 @@ static char const * const for_each_ref_usage[] = {
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
{
- int i, num_refs;
+ int i;
const char *format = %(objectname) %(objecttype)\t%(refname);
struct ref_sort *sort = NULL, **sort_tail = sort;
int maxcount = 0, quote_style = 0;
- struct ref_array_item **refs;
- struct grab_ref_cbdata cbdata;
+ struct ref_filter_cbdata ref_cbdata;
+ memset(ref_cbdata, 0, sizeof(ref_cbdata));
struct option opts[] = {
OPT_BIT('s', shell, quote_style,
@@ -1119,17 +1128,14 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
- memset(cbdata, 0, sizeof(cbdata));
- cbdata.grab_pattern = argv;
- for_each_rawref(grab_single_ref, cbdata);
- refs = cbdata.grab_array;
- num_refs = cbdata.grab_cnt;
+ ref_cbdata.filter.name_patterns = argv;
+ for_each_rawref(grab_single_ref, ref_cbdata);
- sort_refs(sort, refs, num_refs);
+ sort_refs(sort, ref_cbdata.array);
- if (!maxcount || num_refs maxcount)
- maxcount = num_refs;
+ if (!maxcount || ref_cbdata.array.nr maxcount)
+ maxcount = ref_cbdata.array.nr;
for (i
[WIP/PATCH v4 3/8] for-each-ref: rename 'refinfo' to 'ref_array_item'
Rename 'refinfo' to 'ref_array_item' as a preparatory step for introduction of
new structures in the forthcoming patch.
Re-order the fields in 'ref_array_item' so that refname can be
eventually converted to a FLEX_ARRAY.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 919d45e..e634fd2 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -31,12 +31,12 @@ struct ref_sort {
unsigned reverse : 1;
};
-struct refinfo {
- char *refname;
+struct ref_array_item {
unsigned char objectname[20];
int flag;
const char *symref;
struct atom_value *value;
+ char *refname;
};
static struct {
@@ -85,7 +85,7 @@ static struct {
* a * to denote deref_tag().
*
* We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects. refinfo
+ * of properties that we need to extract out of objects. ref_array_item
* structure will hold an array of values extracted that can be
* indexed with the atom number, which is an index into this
* array.
@@ -622,7 +622,7 @@ static inline char *copy_advance(char *dst, const char *src)
/*
* Parse the object referred by ref, and grab needed value.
*/
-static void populate_value(struct refinfo *ref)
+static void populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
@@ -821,7 +821,7 @@ static void populate_value(struct refinfo *ref)
* Given a ref, return the value for the atom. This lazily gets value
* out of the object by calling populate value.
*/
-static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
+static void get_value(struct ref_array_item *ref, int atom, struct atom_value
**v)
{
if (!ref-value) {
populate_value(ref);
@@ -831,7 +831,7 @@ static void get_value(struct refinfo *ref, int atom, struct
atom_value **v)
}
struct grab_ref_cbdata {
- struct refinfo **grab_array;
+ struct ref_array_item **grab_array;
const char **grab_pattern;
int grab_cnt;
};
@@ -860,12 +860,12 @@ static int match_name_as_path(const char **pattern, const
char *refname)
return 0;
}
-/* Allocate space for a new refinfo and copy the objectname and flag to it */
-static struct refinfo *new_refinfo(const char *refname,
- const unsigned char *objectname,
- int flag)
+/* Allocate space for a new ref_array_item and copy the objectname and flag to
it */
+static struct ref_array_item *new_ref_array_item(const char *refname,
+const unsigned char
*objectname,
+int flag)
{
- struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+ struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
ref-refname = xstrdup(refname);
hashcpy(ref-objectname, objectname);
ref-flag = flag;
@@ -880,7 +880,7 @@ static struct refinfo *new_refinfo(const char *refname,
static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
struct grab_ref_cbdata *cb = cb_data;
- struct refinfo *ref;
+ struct ref_array_item *ref;
if (flag REF_BAD_NAME) {
warning(ignoring ref with broken name %s, refname);
@@ -890,7 +890,7 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
if (*cb-grab_pattern !match_name_as_path(cb-grab_pattern, refname))
return 0;
- ref = new_refinfo(refname, sha1, flag);
+ ref = new_ref_array_item(refname, sha1, flag);
REALLOC_ARRAY(cb-grab_array, cb-grab_cnt + 1);
cb-grab_array[cb-grab_cnt++] = ref;
@@ -898,7 +898,7 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
return 0;
}
-static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo
*b)
+static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct
ref_array_item *b)
{
struct atom_value *va, *vb;
int cmp;
@@ -925,8 +925,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct refinfo
*a, struct refinfo *b
static struct ref_sort *ref_sort;
static int compare_refs(const void *a_, const void *b_)
{
- struct refinfo *a = *((struct refinfo **)a_);
- struct refinfo *b = *((struct refinfo **)b_);
+ struct ref_array_item *a = *((struct ref_array_item **)a_);
+ struct ref_array_item *b = *((struct ref_array_item **)b_);
struct ref_sort *s;
for (s = ref_sort; s; s = s-next) {
@@
[WIP/PATCH v4 2/8] for-each-ref: simplify code
In 'grab_single_ref()' remove the extra count variable 'cnt' and
use the variable 'grab_cnt' of structure 'grab_ref_cbdata' directly
instead.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b33e2de..919d45e 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -881,7 +881,6 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
{
struct grab_ref_cbdata *cb = cb_data;
struct refinfo *ref;
- int cnt;
if (flag REF_BAD_NAME) {
warning(ignoring ref with broken name %s, refname);
@@ -893,10 +892,8 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
ref = new_refinfo(refname, sha1, flag);
- cnt = cb-grab_cnt;
- REALLOC_ARRAY(cb-grab_array, cnt + 1);
- cb-grab_array[cnt++] = ref;
- cb-grab_cnt = cnt;
+ REALLOC_ARRAY(cb-grab_array, cb-grab_cnt + 1);
+ cb-grab_array[cb-grab_cnt++] = ref;
return 0;
}
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()
Extract two helper functions out of grab_single_ref(). Firstly,
new_refinfo() which is used to allocate memory for a new refinfo
structure and copy the objectname, refname and flag to it.
Secondly, match_name_as_path() which when given an array of patterns
and the refname checks if the refname matches any of the patterns
given while the pattern is a pathname, also while supporting wild
characters.
This is a preperatory patch for restructuring 'for-each-ref' and
eventually moving most of it to 'ref-filter' to provide the
functionality to similar commands via public API's.
Helped-by: Junio C Hamano gits...@pobox.com
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 69 ++
1 file changed, 41 insertions(+), 28 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..b33e2de 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -837,6 +837,43 @@ struct grab_ref_cbdata {
};
/*
+ * Given a refname, return 1 if the refname matches with one of the patterns
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
+ */
+static int match_name_as_path(const char **pattern, const char *refname)
+{
+ int namelen = strlen(refname);
+ for (; *pattern; pattern++) {
+ const char *p = *pattern;
+ int plen = strlen(p);
+
+ if ((plen = namelen)
+ !strncmp(refname, p, plen)
+ (refname[plen] == '\0' ||
+refname[plen] == '/' ||
+p[plen-1] == '/'))
+ return 1;
+ if (!wildmatch(p, refname, WM_PATHNAME, NULL))
+ return 1;
+ }
+ return 0;
+}
+
+/* Allocate space for a new refinfo and copy the objectname and flag to it */
+static struct refinfo *new_refinfo(const char *refname,
+ const unsigned char *objectname,
+ int flag)
+{
+ struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+ ref-refname = xstrdup(refname);
+ hashcpy(ref-objectname, objectname);
+ ref-flag = flag;
+
+ return ref;
+}
+
+/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
@@ -851,40 +888,16 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
return 0;
}
- if (*cb-grab_pattern) {
- const char **pattern;
- int namelen = strlen(refname);
- for (pattern = cb-grab_pattern; *pattern; pattern++) {
- const char *p = *pattern;
- int plen = strlen(p);
-
- if ((plen = namelen)
- !strncmp(refname, p, plen)
- (refname[plen] == '\0' ||
-refname[plen] == '/' ||
-p[plen-1] == '/'))
- break;
- if (!wildmatch(p, refname, WM_PATHNAME, NULL))
- break;
- }
- if (!*pattern)
- return 0;
- }
+ if (*cb-grab_pattern !match_name_as_path(cb-grab_pattern, refname))
+ return 0;
- /*
-* We do not open the object yet; sort may only need refname
-* to do its job and the resulting list may yet to be pruned
-* by maxcount logic.
-*/
- ref = xcalloc(1, sizeof(*ref));
- ref-refname = xstrdup(refname);
- hashcpy(ref-objectname, sha1);
- ref-flag = flag;
+ ref = new_refinfo(refname, sha1, flag);
cnt = cb-grab_cnt;
REALLOC_ARRAY(cb-grab_array, cnt + 1);
cb-grab_array[cnt++] = ref;
cb-grab_cnt = cnt;
+
return 0;
}
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[WIP/PATCH v4 5/8] for-each-ref: introduce 'ref_filter_clear_data()'
Introduce and implement 'ref_filter_clear_data()' which will free
all allocated memory for 'ref_filter_cbdata' and its underlying array
of 'ref_array_item'.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ef54c90..f896e1c 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -907,6 +907,18 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
return 0;
}
+/* Free all memory allocated for ref_filter_cbdata */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata)
+{
+ int i;
+
+ for (i = 0; i ref_cbdata-array.nr; i++)
+ free(ref_cbdata-array.items[i]);
+ free(ref_cbdata-array.items);
+ ref_cbdata-array.items = NULL;
+ ref_cbdata-array.nr = ref_cbdata-array.alloc = 0;
+}
+
static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct
ref_array_item *b)
{
struct atom_value *va, *vb;
@@ -1137,5 +1149,6 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
maxcount = ref_cbdata.array.nr;
for (i = 0; i maxcount; i++)
show_ref(ref_cbdata.array.items[i], format, quote_style);
+ ref_filter_clear_data(ref_cbdata);
return 0;
}
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
Rename some of the functions and make them publically available.
This is a preparatory step for moving code from 'for-each-ref'
to 'ref-filter' to make meaningful, targeted services available to
other commands via public APIs.
Based-on-patch-by: Jeff King p...@peff.net
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 45 +++--
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f896e1c..8fed04b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -112,7 +112,7 @@ static int need_color_reset_at_eol;
/*
* Used to parse format string and sort specifiers
*/
-static int parse_atom(const char *atom, const char *ep)
+int parse_ref_filter_atom(const char *atom, const char *ep)
{
const char *sp;
int i, at;
@@ -189,7 +189,7 @@ static const char *find_next(const char *cp)
* Make sure the format string is well formed, and parse out
* the used atoms.
*/
-static int verify_format(const char *format)
+int verify_ref_format(const char *format)
{
const char *cp, *sp;
@@ -201,7 +201,7 @@ static int verify_format(const char *format)
if (!ep)
return error(malformed format string %s, sp);
/* sp points at %( and ep points at the closing ) */
- at = parse_atom(sp + 2, ep);
+ at = parse_ref_filter_atom(sp + 2, ep);
cp = ep + 1;
if (skip_prefix(used_atom[at], color:, color))
@@ -408,7 +408,7 @@ static void grab_date(const char *buf, struct atom_value
*v, const char *atomnam
/*
* We got here because atomname ends in date or datesomething;
* it's not possible that something is not :format because
-* parse_atom() wouldn't have allowed it, so we can assume that no
+* parse_ref_filter_atom() wouldn't have allowed it, so we can assume
that no
* : means no format is specified, and use the default.
*/
formatp = strchr(atomname, ':');
@@ -835,7 +835,7 @@ static void populate_value(struct ref_array_item *ref)
* Given a ref, return the value for the atom. This lazily gets value
* out of the object by calling populate value.
*/
-static void get_value(struct ref_array_item *ref, int atom, struct atom_value
**v)
+static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct
atom_value **v)
{
if (!ref-value) {
populate_value(ref);
@@ -882,10 +882,10 @@ static struct ref_array_item *new_ref_array_item(const
char *refname,
}
/*
- * A call-back given to for_each_ref(). Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
struct ref_filter_cbdata *ref_cbdata = cb_data;
struct ref_filter *filter = ref_cbdata-filter;
@@ -925,8 +925,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct
ref_array_item *a, struct ref
int cmp;
cmp_type cmp_type = used_atom_type[s-atom];
- get_value(a, s-atom, va);
- get_value(b, s-atom, vb);
+ get_ref_atom_value(a, s-atom, va);
+ get_ref_atom_value(b, s-atom, vb);
switch (cmp_type) {
case FIELD_STR:
cmp = strcmp(va-s, vb-s);
@@ -958,7 +958,7 @@ static int compare_refs(const void *a_, const void *b_)
return 0;
}
-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array)
{
ref_sort = sort;
qsort(array-items, array-nr, sizeof(struct ref_array_item *),
compare_refs);
@@ -1028,7 +1028,7 @@ static void emit(const char *cp, const char *ep)
}
}
-static void show_ref(struct ref_array_item *info, const char *format, int
quote_style)
+void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
{
const char *cp, *sp, *ep;
@@ -1038,7 +1038,7 @@ static void show_ref(struct ref_array_item *info, const
char *format, int quote_
ep = strchr(sp, ')');
if (cp sp)
emit(cp, sp);
- get_value(info, parse_atom(sp + 2, ep), atomv);
+ get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep),
atomv);
print_value(atomv, quote_style);
}
if (*cp) {
@@ -1057,18 +1057,19 @@ static void show_ref(struct ref_array_item *info, const
char *format, int quote_
putchar('\n');
}
-static struct ref_sort *default_sort(void)
+/* If no sorting option is given, use
[WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
Create 'ref-filter.h', also add ref-filter to the Makefile.
This completes movement of creation of 'ref-filter' from
'for-each-ref'.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
Makefile | 1 +
builtin/for-each-ref.c | 41 +--
ref-filter.c | 1 +
ref-filter.h | 66 ++
4 files changed, 69 insertions(+), 40 deletions(-)
create mode 100644 ref-filter.h
diff --git a/Makefile b/Makefile
index 323c401..ad455a3 100644
--- a/Makefile
+++ b/Makefile
@@ -762,6 +762,7 @@ LIB_OBJS += reachable.o
LIB_OBJS += read-cache.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
+LIB_OBJS += ref-filter.o
LIB_OBJS += remote.o
LIB_OBJS += replace_object.o
LIB_OBJS += rerere.o
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 65ed954..2f11c01 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -2,46 +2,7 @@
#include cache.h
#include refs.h
#include parse-options.h
-
-/* Quoting styles */
-#define QUOTE_NONE 0
-#define QUOTE_SHELL 1
-#define QUOTE_PERL 2
-#define QUOTE_PYTHON 4
-#define QUOTE_TCL 8
-
-struct atom_value {
- const char *s;
- unsigned long ul; /* used for sorting when not FIELD_STR */
-};
-
-struct ref_sort {
- struct ref_sort *next;
- int atom; /* index into used_atom array */
- unsigned reverse : 1;
-};
-
-struct ref_array_item {
- unsigned char objectname[20];
- int flag;
- const char *symref;
- struct atom_value *value;
- char *refname;
-};
-
-struct ref_array {
- int nr, alloc;
- struct ref_array_item **items;
-};
-
-struct ref_filter {
- const char **name_patterns;
-};
-
-struct ref_filter_cbdata {
- struct ref_array array;
- struct ref_filter filter;
-};
+#include ref-filter.h
static char const * const for_each_ref_usage[] = {
N_(git for-each-ref [options] [pattern]),
diff --git a/ref-filter.c b/ref-filter.c
index 2a8f504..1c73750 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2,6 +2,7 @@
#include cache.h
#include parse-options.h
#include refs.h
+#include ref-filter.h
#include wildmatch.h
#include commit.h
#include remote.h
diff --git a/ref-filter.h b/ref-filter.h
new file mode 100644
index 000..dacae59
--- /dev/null
+++ b/ref-filter.h
@@ -0,0 +1,66 @@
+#ifndef REF_FILTER_H
+#define REF_FILTER_H
+
+#include sha1-array.h
+#include refs.h
+#include commit.h
+#include parse-options.h
+
+/* Quoting styles */
+#define QUOTE_NONE 0
+#define QUOTE_SHELL 1
+#define QUOTE_PERL 2
+#define QUOTE_PYTHON 4
+#define QUOTE_TCL 8
+
+struct atom_value {
+ const char *s;
+ unsigned long ul; /* used for sorting when not FIELD_STR */
+};
+
+struct ref_sort {
+ struct ref_sort *next;
+ int atom; /* index into used_atom array */
+ unsigned reverse : 1;
+};
+
+struct ref_array_item {
+ unsigned char objectname[20];
+ int flag;
+ const char *symref;
+ struct atom_value *value;
+ char *refname;
+};
+
+struct ref_array {
+ int nr, alloc;
+ struct ref_array_item **items;
+};
+
+struct ref_filter {
+ const char **name_patterns;
+};
+
+struct ref_filter_cbdata {
+ struct ref_array array;
+ struct ref_filter filter;
+};
+
+/* Callback function for for_each_*ref(). This filters the refs based on the
filters set */
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int
flag, void *cb_data);
+/* Clear all memory allocated to ref_filter_data */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata);
+/* Parse format string and sort specifiers */
+int parse_ref_filter_atom(const char *atom, const char *ep);
+/* Used to verify if the given format is correct and to parse out the used
atoms */
+int verify_ref_format(const char *format);
+/* Sort the given ref_array as per the ref_sort provided */
+void sort_ref_array(struct ref_sort *sort, struct ref_array *array);
+/* Print the ref using the given format and quote_style */
+void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style);
+/* Callback function for parsing the sort option */
+int opt_parse_ref_sort(const struct option *opt, const char *arg, int unset);
+/* Default sort option based on refname */
+struct ref_sort *ref_default_sort(void);
+
+#endif /* REF_FILTER_H */
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[WIP/PATCH v4 7/8] ref-filter: move code from 'for-each-ref'
Move most of the code from 'for-each-ref' to 'ref-filter' to make
it publicly available to other commands, this is to unify the code
of 'tag -l', 'branch -l' and 'for-each-ref' so that they can share
their implementations with each other.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 1048 ---
ref-filter.c | 1050
2 files changed, 1050 insertions(+), 1048 deletions(-)
create mode 100644 ref-filter.c
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 8fed04b..65ed954 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -1,15 +1,7 @@
#include builtin.h
#include cache.h
#include refs.h
-#include object.h
-#include tag.h
-#include commit.h
-#include tree.h
-#include blob.h
-#include quote.h
#include parse-options.h
-#include remote.h
-#include color.h
/* Quoting styles */
#define QUOTE_NONE 0
@@ -18,8 +10,6 @@
#define QUOTE_PYTHON 4
#define QUOTE_TCL 8
-typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
-
struct atom_value {
const char *s;
unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -53,1044 +43,6 @@ struct ref_filter_cbdata {
struct ref_filter filter;
};
-static struct {
- const char *name;
- cmp_type cmp_type;
-} valid_atom[] = {
- { refname },
- { objecttype },
- { objectsize, FIELD_ULONG },
- { objectname },
- { tree },
- { parent },
- { numparent, FIELD_ULONG },
- { object },
- { type },
- { tag },
- { author },
- { authorname },
- { authoremail },
- { authordate, FIELD_TIME },
- { committer },
- { committername },
- { committeremail },
- { committerdate, FIELD_TIME },
- { tagger },
- { taggername },
- { taggeremail },
- { taggerdate, FIELD_TIME },
- { creator },
- { creatordate, FIELD_TIME },
- { subject },
- { body },
- { contents },
- { contents:subject },
- { contents:body },
- { contents:signature },
- { upstream },
- { symref },
- { flag },
- { HEAD },
- { color },
-};
-
-/*
- * An atom is a valid field atom listed above, possibly prefixed with
- * a * to denote deref_tag().
- *
- * We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects. ref_array_item
- * structure will hold an array of values extracted that can be
- * indexed with the atom number, which is an index into this
- * array.
- */
-static const char **used_atom;
-static cmp_type *used_atom_type;
-static int used_atom_cnt, need_tagged, need_symref;
-static int need_color_reset_at_eol;
-
-/*
- * Used to parse format string and sort specifiers
- */
-int parse_ref_filter_atom(const char *atom, const char *ep)
-{
- const char *sp;
- int i, at;
-
- sp = atom;
- if (*sp == '*' sp ep)
- sp++; /* deref */
- if (ep = sp)
- die(malformed field name: %.*s, (int)(ep-atom), atom);
-
- /* Do we have the atom already used elsewhere? */
- for (i = 0; i used_atom_cnt; i++) {
- int len = strlen(used_atom[i]);
- if (len == ep - atom !memcmp(used_atom[i], atom, len))
- return i;
- }
-
- /* Is the atom a valid one? */
- for (i = 0; i ARRAY_SIZE(valid_atom); i++) {
- int len = strlen(valid_atom[i].name);
- /*
-* If the atom name has a colon, strip it and everything after
-* it off - it specifies the format for this entry, and
-* shouldn't be used for checking against the valid_atom
-* table.
-*/
- const char *formatp = strchr(sp, ':');
- if (!formatp || ep formatp)
- formatp = ep;
- if (len == formatp - sp !memcmp(valid_atom[i].name, sp, len))
- break;
- }
-
- if (ARRAY_SIZE(valid_atom) = i)
- die(unknown field name: %.*s, (int)(ep-atom), atom);
-
- /* Add it in, including the deref prefix */
- at = used_atom_cnt;
- used_atom_cnt++;
- REALLOC_ARRAY(used_atom, used_atom_cnt);
- REALLOC_ARRAY(used_atom_type, used_atom_cnt);
- used_atom[at] = xmemdupz(atom, ep - atom);
- used_atom_type[at] = valid_atom[i].cmp_type;
- if (*atom == '*')
- need_tagged = 1;
- if (!strcmp(used_atom[at], symref))
- need_symref = 1;
- return at;
-}
-
-/*
- * In a format string, find the next occurrence of %(atom).
- */
-static const char *find_next(const char *cp)
-{
- while (*cp) {
- if
Re: [PATCH v2 1/2] completion: Add sequencer function
Quoting Thomas Braun thomas.br...@virtuell-zuhause.de:
Signed-off-by: Thomas Braun thomas.br...@virtuell-zuhause.de
---
contrib/completion/git-completion.bash | 48
+++---
1 file changed, 33 insertions(+), 15 deletions(-)
I don't see the benefits of this change. This patch adds more than
twice as many lines as it removes, and patch 2/2 adds 8 new lines
although it could get away with only 5 without this function. To
offer sequencer options we currently go through a single if statement,
with this patch we'd go through a case statement, an if statement and
finally an .
Gábor
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index bfc74e9..f6e5bf6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -851,15 +851,40 @@ __git_count_arguments ()
printf %d $c
}
+__git_complete_sequencer ()
+{
+ local dir=$(__gitdir)
+
+ case $1 in
+ am)
+ if [ -d $dir/rebase-apply ]; then
+ __gitcomp --skip --continue --resolved --abort
+ return 0
+ fi
+ ;;
+ cherry-pick)
+ if [ -f $dir/CHERRY_PICK_HEAD ]; then
+ __gitcomp --continue --quit --abort
+ return 0
+ fi
+ ;;
+ rebase)
+ if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ];
then
+ __gitcomp --continue --skip --abort
+ return 0
+ fi
+ ;;
+ esac
+
+ return 1
+}
+
__git_whitespacelist=nowarn warn error error-all fix
_git_am ()
{
- local dir=$(__gitdir)
- if [ -d $dir/rebase-apply ]; then
- __gitcomp --skip --continue --resolved --abort
- return
- fi
+ __git_complete_sequencer am return
+
case $cur in
--whitespace=*)
__gitcomp $__git_whitespacelist ${cur##--whitespace=}
@@ -1044,11 +1069,8 @@ _git_cherry ()
_git_cherry_pick ()
{
- local dir=$(__gitdir)
- if [ -f $dir/CHERRY_PICK_HEAD ]; then
- __gitcomp --continue --quit --abort
- return
- fi
+ __git_complete_sequencer cherry-pick return
+
case $cur in
--*)
__gitcomp --edit --no-commit --signoff --strategy= --mainline
@@ -1666,11 +1688,7 @@ _git_push ()
_git_rebase ()
{
- local dir=$(__gitdir)
- if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ]; then
- __gitcomp --continue --skip --abort
- return
- fi
+ __git_complete_sequencer rebase return
__git_complete_strategy return
case $cur in
--whitespace=*)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Diffing submodule does not yield complete logs for merge commits
On Sat, May 30, 2015 at 2:19 PM, Robert Dailey rcdailey.li...@gmail.com wrote: On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano gits...@pobox.com wrote: Robert Dailey rcdailey.li...@gmail.com writes: In the meantime I'd like to ask, do we even need to add an option for this? What if we just make `diff.submodule log` not use --first-parent? This seems like a backward compatible change in of itself. Why? People have relied on submodule-log not to include all the noise coming from individual commits on side branches and instead appreciated seeing only the overview by merges of side branch topics being listed---why is regressing the system to inconvenience these existing users a backward compatible change? Backward compatible in the sense that it does not break existing functionality. For example, removing -D from `git branch` would be a backward breaking change. Also not everyone has a disciplined merge-commit editing policy like the Linux Git projects do. In fact (especially in corporate environments), most merge commit messages are useless and auto-generated. It's in fact very common (depending on popular tooling) to not have the ability to edit these messages. Example: Merge branch topic into master This is the defacto message you get when you use Github, Atlassian Stash, etc. any time you merge a PR. For repositories that only accept changes through pull requests, it is not possible to generate a diff log of submodules that is useful without showing the ancestor commits on all parents. Right now 'log' literally gives you the following for a submodule that has a mainline branch that does not accept direct pushes (i.e. only PRs): Merge branch topic1 into master Merge branch topic2 into master Merge branch origin/develop into master Merge branch topic3 into master I would argue that this situation is common -- more common than what you would typically see in a well groomed git repository that does not use PRs or always does rebase+ff-merge. That is the basis for my concern. No functionality is broken and cators to the common case. But I guess since we're discussing 2 distinct workflows, it makes sense to have 2 options for viewing the submodule logs. Because if 'full-log' were indeed the default, it would cause a lot of noise in the well-disciplined-merge-commit workflow. From a high level (to explain my motivation for my simplified and arguably naive suggestion), I work with a lot of amateur users of Git. They always complain about using Git and how SVN is better. Yet they do not accept the challenge of learning Git, which is a very complicated tool. Much of git I would argue is not very beginner (even user) friendly (although things are certainly getting better). With such an advanced tool, with such complex configuration and behavior, and given all the friction it causes, I can only do my best to offer seemingly sensible alternatives to adding MORE configuration. Anyway it's just a thought. Glad to get feedback and I see both sides of the fence on this one. And it's simpler to implement. I can't think of a good justification to add more settings to an already hugely complex configuration scheme for such a minor difference in behavior. Careful, as that argument can cut both ways. If it is so a minor difference in behaviour, perhaps we can do without not just an option but a feature to omit --first-parent here. That would be even simpler to implement, as you do not have to do anything. So, if you think the new behaviour can help _some_ users, then you would need the feature and a knob to enable it, I would think. I don't really understand your contrasted example here. Can you explain: ...we can do without not just an option but a feature to omit --first-parent... Again thanks for the feedback. I'm having some difficulty with the tests. What it looks like is that the test repository is created by calling test-lib.sh at the top. Then by the time the commands after that are run, it's inside the temp repository. At the bottom of t4041, I add this: test_create_repo sm3 git add sm3 cd sm3 echo foo.txt git add foo.txt git commit -m 'foo.txt' /dev/null git checkout -b topic /dev/null echo topic.txt git add topic.txt git commit -m 'topic.txt' /dev/null git checkout master /dev/null git merge --no-ff --no-edit topic /dev/null cd .. git diff --submodule=log sm3 But this does not work as I expect. How do I add a submodule and then simulate creation of branches, commits, and merges, prior to running code in text_expect_success? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Diffing submodule does not yield complete logs for merge commits
Robert Dailey rcdailey.li...@gmail.com writes: On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano gits...@pobox.com wrote: Robert Dailey rcdailey.li...@gmail.com writes: In the meantime I'd like to ask, do we even need to add an option for this? What if we just make `diff.submodule log` not use --first-parent? This seems like a backward compatible change in of itself. Why? People have relied on submodule-log not to include all the noise coming from individual commits on side branches and instead appreciated seeing only the overview by merges of side branch topics being listed---why is regressing the system to inconvenience these existing users a backward compatible change? Backward compatible in the sense that it does not break existing functionality And adding one-line-per-commit from side branches does break existing functionality. You seem to be arguing that more information is always good and does not break existing functionality, but summarizing by omitting irrelevant details *is* a feature. Do you honestly believe that a change to make the full log -p output in submodule log be a backward compatible change?? Merge branch topic1 into master Merge branch topic2 into master Merge branch origin/develop into master Merge branch topic3 into master It is not a real argument; it is something you can fix by naming your branches more sensibly, which would make you a better developer regardless of how submodule-log is shown. And it's simpler to implement. I can't think of a good justification to add more settings to an already hugely complex configuration scheme for such a minor difference in behavior. Careful, as that argument can cut both ways. If it is so a minor difference in behaviour, perhaps we can do without not just an option but a feature to omit --first-parent here. That would be even simpler to implement, as you do not have to do anything. I don't really understand your contrasted example here. Can you explain: ...we can do without not just an option but a feature to omit --first-parent... I am not sure how to phrase that any easier. Sorry. Assuming that you consider output with and without --first-parent does not make much of a difference (i.e. minor difference in behaviour), instead of dropping --first-parent unconditionally, like you seem to be pushing with the argument, we can unconditionally keep the --first-parent and do not change anything. The end result would not make much of a difference either way, and not doing anything is much simpler than dropping --first-parent. Hopefully you can see how absurd that line of reasoning is. So do not make the same argument to push for changing the behaviour unconditionally. If you think the new behaviour can help _some_ users, then you would need the feature and a knob to enable it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame: add blame.showemail config option
On Fri, May 29, 2015 at 2:40 PM, Junio C Hamano gits...@pobox.com wrote: Quentin Neill quentin.ne...@gmail.com writes: Thanks for the thorough review! I have adjusted the commit messages and updated the documentation changes. I'm in trying to add tests, I'll probably have some issues but will post something that works soon. Hi, I was sweeping my old mailbox for leftover bits, and noticed that this thread ended without seeing the final step. Has anything further happened to this topic, did you got too busy, or you lost interest? [One more time without html] Hi Junio, I got too busy, my free time went to zero. And this week I am traveling. I was tinkering with tests Thursday night and was in fact preparing a reply. When I get back I'll post the patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] commit: fix ending newline for template files
Eric Sunshine sunsh...@sunshineco.com writes: On Fri, May 29, 2015 at 4:17 PM, Junio C Hamano gits...@pobox.com wrote: By default, we should run clean-up after the editor we spawned gives us the edited result. Not adding one more LF after the template when it already ends with LF would not hurt, but an extra blank after the template material does not hurt, either, so I am honestly indifferent. I had a similar reaction. The one salient bit I picked up was that Patryk finds it aesthetically offensive[1] when the template ends with a comment line, and that comment line does not flow directly into the comment lines provided by --status. That is: Template line 1 # Template line 2 # Please enter the commit message... # with '#' will be ignored... [1]: Quoting from the commit message of patch 1/2: ...which is very annoying when template ends with line starting with '#' As I said in the message you are responding to, I do not think it would hurt if we stopped adding an LF after a template that already ends with LF. I think I am OK with a patch that does so without doing anything else, like changing when clean-up happens, etc. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Diffing submodule does not yield complete logs for merge commits
Robert Dailey rcdailey.li...@gmail.com writes: In the meantime I'd like to ask, do we even need to add an option for this? What if we just make `diff.submodule log` not use --first-parent? This seems like a backward compatible change in of itself. Why? People have relied on submodule-log not to include all the noise coming from individual commits on side branches and instead appreciated seeing only the overview by merges of side branch topics being listed---why is regressing the system to inconvenience these existing users a backward compatible change? And it's simpler to implement. I can't think of a good justification to add more settings to an already hugely complex configuration scheme for such a minor difference in behavior. Careful, as that argument can cut both ways. If it is so a minor difference in behaviour, perhaps we can do without not just an option but a feature to omit --first-parent here. That would be even simpler to implement, as you do not have to do anything. So, if you think the new behaviour can help _some_ users, then you would need the feature and a knob to enable it, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[WIP/PATCH v4 0/8] ref-filter: move code from for-each-ref
Hello, After the third iteration of this WIP/PATCH series ($gmane/270164). This is a follow up. Changes, * Subdivided some patches. * Spelling corrections. * Small changes. Thanks all for suggestions. -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Diffing submodule does not yield complete logs for merge commits
On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano gits...@pobox.com wrote: Robert Dailey rcdailey.li...@gmail.com writes: In the meantime I'd like to ask, do we even need to add an option for this? What if we just make `diff.submodule log` not use --first-parent? This seems like a backward compatible change in of itself. Why? People have relied on submodule-log not to include all the noise coming from individual commits on side branches and instead appreciated seeing only the overview by merges of side branch topics being listed---why is regressing the system to inconvenience these existing users a backward compatible change? Backward compatible in the sense that it does not break existing functionality. For example, removing -D from `git branch` would be a backward breaking change. Also not everyone has a disciplined merge-commit editing policy like the Linux Git projects do. In fact (especially in corporate environments), most merge commit messages are useless and auto-generated. It's in fact very common (depending on popular tooling) to not have the ability to edit these messages. Example: Merge branch topic into master This is the defacto message you get when you use Github, Atlassian Stash, etc. any time you merge a PR. For repositories that only accept changes through pull requests, it is not possible to generate a diff log of submodules that is useful without showing the ancestor commits on all parents. Right now 'log' literally gives you the following for a submodule that has a mainline branch that does not accept direct pushes (i.e. only PRs): Merge branch topic1 into master Merge branch topic2 into master Merge branch origin/develop into master Merge branch topic3 into master I would argue that this situation is common -- more common than what you would typically see in a well groomed git repository that does not use PRs or always does rebase+ff-merge. That is the basis for my concern. No functionality is broken and cators to the common case. But I guess since we're discussing 2 distinct workflows, it makes sense to have 2 options for viewing the submodule logs. Because if 'full-log' were indeed the default, it would cause a lot of noise in the well-disciplined-merge-commit workflow. From a high level (to explain my motivation for my simplified and arguably naive suggestion), I work with a lot of amateur users of Git. They always complain about using Git and how SVN is better. Yet they do not accept the challenge of learning Git, which is a very complicated tool. Much of git I would argue is not very beginner (even user) friendly (although things are certainly getting better). With such an advanced tool, with such complex configuration and behavior, and given all the friction it causes, I can only do my best to offer seemingly sensible alternatives to adding MORE configuration. Anyway it's just a thought. Glad to get feedback and I see both sides of the fence on this one. And it's simpler to implement. I can't think of a good justification to add more settings to an already hugely complex configuration scheme for such a minor difference in behavior. Careful, as that argument can cut both ways. If it is so a minor difference in behaviour, perhaps we can do without not just an option but a feature to omit --first-parent here. That would be even simpler to implement, as you do not have to do anything. So, if you think the new behaviour can help _some_ users, then you would need the feature and a knob to enable it, I would think. I don't really understand your contrasted example here. Can you explain: ...we can do without not just an option but a feature to omit --first-parent... Again thanks for the feedback. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Diffing submodule does not yield complete logs for merge commits
On Sat, May 30, 2015 at 2:54 PM, Junio C Hamano gits...@pobox.com wrote: Robert Dailey rcdailey.li...@gmail.com writes: On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano gits...@pobox.com wrote: Robert Dailey rcdailey.li...@gmail.com writes: In the meantime I'd like to ask, do we even need to add an option for this? What if we just make `diff.submodule log` not use --first-parent? This seems like a backward compatible change in of itself. Why? People have relied on submodule-log not to include all the noise coming from individual commits on side branches and instead appreciated seeing only the overview by merges of side branch topics being listed---why is regressing the system to inconvenience these existing users a backward compatible change? Backward compatible in the sense that it does not break existing functionality And adding one-line-per-commit from side branches does break existing functionality. You seem to be arguing that more information is always good and does not break existing functionality, but summarizing by omitting irrelevant details *is* a feature. Do you honestly believe that a change to make the full log -p output in submodule log be a backward compatible change?? Merge branch topic1 into master Merge branch topic2 into master Merge branch origin/develop into master Merge branch topic3 into master It is not a real argument; it is something you can fix by naming your branches more sensibly, which would make you a better developer regardless of how submodule-log is shown. I just use git, I don't have the deep technical understanding of its implementation as you may have. From my perspective I can't think of how this breaks backward compatibility, or perhaps your definition of backward compatibility does not align with mine. And please don't over generalize and misconvey what I said. I am not saying more information is always good. Did you not read anything I wrote? Also good branch names may help but that doesn't go into detail and explain features that may have been sitting on a topic branch for weeks. That's not a practical solution to the problem. Also branch names do not determine or influence the skill and quality of a developer, as you seem to imply. I'll do us both a favor and end the discussion here, as I do not feel you are being very patient or welcoming in the discussion. I sense frustration on your side. And it's simpler to implement. I can't think of a good justification to add more settings to an already hugely complex configuration scheme for such a minor difference in behavior. Careful, as that argument can cut both ways. If it is so a minor difference in behaviour, perhaps we can do without not just an option but a feature to omit --first-parent here. That would be even simpler to implement, as you do not have to do anything. I don't really understand your contrasted example here. Can you explain: ...we can do without not just an option but a feature to omit --first-parent... I am not sure how to phrase that any easier. Sorry. You mean you don't want to? That's fine, if you don't have the will or patience to explain then I won't bother caring. Assuming that you consider output with and without --first-parent does not make much of a difference (i.e. minor difference in behaviour), instead of dropping --first-parent unconditionally, like you seem to be pushing with the argument, we can unconditionally keep the --first-parent and do not change anything. The end result would not make much of a difference either way, and not doing anything is much simpler than dropping --first-parent. Hopefully you can see how absurd that line of reasoning is. So do not make the same argument to push for changing the behaviour unconditionally. If you think the new behaviour can help _some_ users, then you would need the feature and a knob to enable it. First of all, you keep calling this an argument. Perhaps it is for you, since you're being absurdly rude with me and impatient with the discussion. This is a brainstorming session. My suggestions may not seem rational or make sense, but this is natural since I am ignorant of the finer details of the application. You're really just overanalyzing my statements from a nonsensical perspective. I am talking about not adding more settings to an already complex set of settings. I am not justifying my feature. I think the feature is just as justified as everything else. Git is FULL of tons of little options to cater to niche workflows. I am not fighting against having another option. In fact, that was my idea to begin with. I am investigating and trying to discuss all possible approaches and perspectives. Your attitude is not very welcoming to those that wish to contribute to the project. In fact, because of your attitude towards me, I will kindly see myself out. I do not have time to spend my free time dealing with this nonsense and irrational
[PATCH mh/lockfile-retry] lockfile: replace random() by rand()
On Windows, we do not have functions srandom() and random(). Use srand()
and rand(). These functions produce random numbers of lesser quality,
but for the purpose (a retry time-out) they are still good enough.
Signed-off-by: Johannes Sixt j...@kdbg.org
---
There you have it: Look the other way for a while, and people start
using exotic stuff... ;)
This is a build breakage of master on Windows. There are also a few
new test suite failures. On of them is in t1404#2, indicating that
a DF conflict takes a different error path. I haven't debugged, yet.
The lock file retry test fails, too. I'll report back as time permits.
lockfile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index 5a93bc7..ee5cb01 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -191,7 +191,7 @@ static int lock_file_timeout(struct lock_file *lk, const
char *path,
return lock_file(lk, path, flags);
if (!random_initialized) {
- srandom((unsigned int)getpid());
+ srand((unsigned int)getpid());
random_initialized = 1;
}
@@ -218,7 +218,7 @@ static int lock_file_timeout(struct lock_file *lk, const
char *path,
backoff_ms = multiplier * INITIAL_BACKOFF_MS;
/* back off for between 0.75*backoff_ms and 1.25*backoff_ms */
- wait_us = (750 + random() % 500) * backoff_ms;
+ wait_us = (750 + rand() % 500) * backoff_ms;
sleep_microseconds(wait_us);
remaining_us -= wait_us;
--
2.3.2.245.gb5bf9d3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] commit: fix ending newline for template files
On Sat, May 30, 2015 at 7:29 AM, Patryk Obara patryk.ob...@gmail.com wrote: On Thu, May 28, 2015 at 4:29 PM, Eric Sunshine sunsh...@sunshineco.com wrote: Did you consider the alternate approach of handling newline processing immediately upon loading 'logfile' and 'template_file', rather than delaying processing until this point? Doing it that way would involve a bit of code repetition but might be easier to reason about since it would occur before possible interactions in following code (such as --signoff handling). Yes. I opted to place it in here, because newline was appended previously also in if (use_editor) block. But I agree, appending this newline after loading file will be cleaner - and code repetition may be avoided, if I'll separate file loading code into new function. A need for this sort of functionality has come up before, so it might be reasonable to introduce a new strbuf function for appending a character if missing. In addition to the 'newline' case, appending '/' to a pathname is also somewhat common. On Sat, May 30, 2015 at 12:25 AM, Eric Sunshine sunsh...@sunshineco.com wrote: If the user specified with the --cleanup option not to clean-up the result coming back from the editor, then the commented material needs to be removed in the editor by the user *anyway*. You misattributed this statement. It was from Junio, not I. Why? Is it not ok to leave lines starting with hash in commit object? --cleanup=whitespace|verbatim suggests, that it's a valid usecase. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] blame: add blame.showEmail configuration
From: Quentin Neill quentin.ne...@gmail.com
Complement existing --show-email option with fallback
configuration variable, with tests.
---
Documentation/git-blame.txt | 2 ++
builtin/blame.c | 10 +++-
t/t8002-blame.sh| 62 +
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 9f23a86..e6e947c 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -76,6 +76,8 @@ include::blame-options.txt[]
-e::
--show-email::
Show the author email instead of author name (Default: off).
+ This can also be controlled via the `blame.showEmail` config
+ option.
-w::
Ignore whitespace when comparing the parent's version and
diff --git a/builtin/blame.c b/builtin/blame.c
index 8d70623..60039d3 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2185,6 +2185,14 @@ static int git_blame_config(const char *var, const char
*value, void *cb)
blank_boundary = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, blame.showemail)) {
+ int *output_option = cb;
+ if (git_config_bool(var, value))
+ *output_option |= OUTPUT_SHOW_EMAIL;
+ else
+ *output_option = ~OUTPUT_SHOW_EMAIL;
+ return 0;
+ }
if (!strcmp(var, blame.date)) {
if (!value)
return config_error_nonbool(var);
@@ -2529,7 +2537,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
unsigned int range_i;
long anchor;
- git_config(git_blame_config, NULL);
+ git_config(git_blame_config, output_option);
init_revisions(revs, NULL);
revs.date_mode = blame_date_mode;
DIFF_OPT_SET(revs.diffopt, ALLOW_TEXTCONV);
diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
index 5cdf3f1..faf1660 100755
--- a/t/t8002-blame.sh
+++ b/t/t8002-blame.sh
@@ -19,4 +19,66 @@ test_expect_success 'blame --show-email' '
E at test dot git 1
'
+test_expect_success 'setup showEmail tests' '
+ echo bin: test number 1 one
+ git add .
+ GIT_AUTHOR_NAME=name1 GIT_AUTHOR_EMAIL=ema...@test.git git commit -a -m
First --date=2010-01-01 01:00:00
+'
+
+cat expected_n \EOF
+(name1 2010-01-01 01:00:00 + 1) bin: test number 1
+EOF
+
+cat expected_e \EOF
+(ema...@test.git 2010-01-01 01:00:00 + 1) bin: test number 1
+EOF
+
+find_blame() {
+ sed -e 's/^[^(]*//'
+}
+
+test_expect_success 'blame with no options and no config' '
+ git blame one blame
+ find_blame blame result
+ test_cmp expected_n result
+'
+
+test_expect_success 'blame with showemail options' '
+ git blame --show-email one blame1
+ find_blame blame1 result
+ test_cmp expected_e result
+ git blame -e one blame2
+ find_blame blame2 result
+ test_cmp expected_e result
+ git blame --no-show-email one blame3
+ find_blame blame3 result
+ test_cmp expected_n result
+'
+
+test_expect_success 'blame with showEmail config false' '
+ git config blame.showEmail false
+ git blame one blame1
+ find_blame blame1 result
+ test_cmp expected_n result
+ git blame --show-email one blame2
+ find_blame blame2 result
+ test_cmp expected_e result
+ git blame -e one blame3
+ find_blame blame3 result
+ test_cmp expected_e result
+ git blame --no-show-email one blame4
+ find_blame blame4 result
+ test_cmp expected_n result
+'
+
+test_expect_success 'blame with showEmail config true' '
+ git config blame.showEmail true
+ git blame one blame1
+ find_blame blame1 result
+ test_cmp expected_e result
+ git blame --no-show-email one blame2
+ find_blame blame2 result
+ test_cmp expected_n result
+'
+
test_done
--
2.4.2.340.g5ecd853
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'
On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak karthik@gmail.com wrote:
Create 'ref-filter.h', also add ref-filter to the Makefile.
This completes movement of creation of 'ref-filter' from
'for-each-ref'.
It's important that the project can be built successfully and function
correctly at each stage of a patch series. Unfortunately, the way this
series is organized, the build breaks after application of patch 7/8
since for-each-ref.c is trying to access functionality which was moved
to ref-filter.c, but there is no header at that stage which advertises
the functionality. Fixing this may be as simple as swapping patches
7/8 and 8/8, along with whatever minor adjustments they might need to
keep them sane. (The alternative would be to combine patches 7/8 and
8/8, however, Matthieu didn't seem to favor that approach[1].)
Overall, this round has been a more pleasant read than previous rounds.
[1]: http://article.gmane.org/gmane.comp.version-control.git/270192
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
Makefile | 1 +
builtin/for-each-ref.c | 41 +--
ref-filter.c | 1 +
ref-filter.h | 66
++
4 files changed, 69 insertions(+), 40 deletions(-)
create mode 100644 ref-filter.h
diff --git a/Makefile b/Makefile
index 323c401..ad455a3 100644
--- a/Makefile
+++ b/Makefile
@@ -762,6 +762,7 @@ LIB_OBJS += reachable.o
LIB_OBJS += read-cache.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
+LIB_OBJS += ref-filter.o
LIB_OBJS += remote.o
LIB_OBJS += replace_object.o
LIB_OBJS += rerere.o
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 65ed954..2f11c01 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -2,46 +2,7 @@
#include cache.h
#include refs.h
#include parse-options.h
-
-/* Quoting styles */
-#define QUOTE_NONE 0
-#define QUOTE_SHELL 1
-#define QUOTE_PERL 2
-#define QUOTE_PYTHON 4
-#define QUOTE_TCL 8
-
-struct atom_value {
- const char *s;
- unsigned long ul; /* used for sorting when not FIELD_STR */
-};
-
-struct ref_sort {
- struct ref_sort *next;
- int atom; /* index into used_atom array */
- unsigned reverse : 1;
-};
-
-struct ref_array_item {
- unsigned char objectname[20];
- int flag;
- const char *symref;
- struct atom_value *value;
- char *refname;
-};
-
-struct ref_array {
- int nr, alloc;
- struct ref_array_item **items;
-};
-
-struct ref_filter {
- const char **name_patterns;
-};
-
-struct ref_filter_cbdata {
- struct ref_array array;
- struct ref_filter filter;
-};
+#include ref-filter.h
static char const * const for_each_ref_usage[] = {
N_(git for-each-ref [options] [pattern]),
diff --git a/ref-filter.c b/ref-filter.c
index 2a8f504..1c73750 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2,6 +2,7 @@
#include cache.h
#include parse-options.h
#include refs.h
+#include ref-filter.h
#include wildmatch.h
#include commit.h
#include remote.h
diff --git a/ref-filter.h b/ref-filter.h
new file mode 100644
index 000..dacae59
--- /dev/null
+++ b/ref-filter.h
@@ -0,0 +1,66 @@
+#ifndef REF_FILTER_H
+#define REF_FILTER_H
+
+#include sha1-array.h
+#include refs.h
+#include commit.h
+#include parse-options.h
+
+/* Quoting styles */
+#define QUOTE_NONE 0
+#define QUOTE_SHELL 1
+#define QUOTE_PERL 2
+#define QUOTE_PYTHON 4
+#define QUOTE_TCL 8
+
+struct atom_value {
+ const char *s;
+ unsigned long ul; /* used for sorting when not FIELD_STR */
+};
+
+struct ref_sort {
+ struct ref_sort *next;
+ int atom; /* index into used_atom array */
+ unsigned reverse : 1;
+};
+
+struct ref_array_item {
+ unsigned char objectname[20];
+ int flag;
+ const char *symref;
+ struct atom_value *value;
+ char *refname;
+};
+
+struct ref_array {
+ int nr, alloc;
+ struct ref_array_item **items;
+};
+
+struct ref_filter {
+ const char **name_patterns;
+};
+
+struct ref_filter_cbdata {
+ struct ref_array array;
+ struct ref_filter filter;
+};
+
+/* Callback function for for_each_*ref(). This filters the refs based on
the filters set */
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int
flag, void *cb_data);
+/* Clear all memory allocated to ref_filter_data */
+void ref_filter_clear_data(struct ref_filter_cbdata *ref_cbdata);
+/* Parse format string and sort specifiers */
+int parse_ref_filter_atom(const char *atom, const char *ep);
+/* Used to verify if the given format is correct and to parse out the used
atoms */
+int verify_ref_format(const char *format);
+/* Sort the given
Re: [WIP/PATCH v4 4/8] for-each-ref: introduce new structures for better organisation
On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak karthik@gmail.com wrote:
Intoduce 'ref_filter_cbdata' which will hold 'ref_filter'
s/Intoduce/Introduce/
(Conditions to filter the refs on) and 'ref_array' (The array
s/Conditions/conditions/
s/The/the/
of ref_array_items). Modify the code to use these new structures.
This is a preparatory patch to eventually move code from 'for-each-ref'
to 'ref-filter' and making it publically available.
s/publically/publicly/
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e634fd2..ef54c90 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -85,7 +99,7 @@ static struct {
* a * to denote deref_tag().
*
* We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects. ref_array_item
+ * of properties that we need to extract out of objects. ref_array_item
Sneaking in whitespace change?
* structure will hold an array of values extracted that can be
* indexed with the atom number, which is an index into this
* array.
@@ -1076,12 +1085,12 @@ static char const * const for_each_ref_usage[] = {
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
{
- int i, num_refs;
+ int i;
const char *format = %(objectname) %(objecttype)\t%(refname);
struct ref_sort *sort = NULL, **sort_tail = sort;
int maxcount = 0, quote_style = 0;
- struct ref_array_item **refs;
- struct grab_ref_cbdata cbdata;
+ struct ref_filter_cbdata ref_cbdata;
+ memset(ref_cbdata, 0, sizeof(ref_cbdata));
struct option opts[] = {
Declaration (struct option opts[]) after statement (memset). I think
you want to leave the memset() where it was below.
OPT_BIT('s', shell, quote_style,
@@ -1119,17 +1128,14 @@ int cmd_for_each_ref(int argc, const char **argv,
const char *prefix)
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
- memset(cbdata, 0, sizeof(cbdata));
- cbdata.grab_pattern = argv;
- for_each_rawref(grab_single_ref, cbdata);
- refs = cbdata.grab_array;
- num_refs = cbdata.grab_cnt;
+ ref_cbdata.filter.name_patterns = argv;
+ for_each_rawref(grab_single_ref, ref_cbdata);
- sort_refs(sort, refs, num_refs);
+ sort_refs(sort, ref_cbdata.array);
- if (!maxcount || num_refs maxcount)
- maxcount = num_refs;
+ if (!maxcount || ref_cbdata.array.nr maxcount)
+ maxcount = ref_cbdata.array.nr;
for (i = 0; i maxcount; i++)
- show_ref(refs[i], format, quote_style);
+ show_ref(ref_cbdata.array.items[i], format, quote_style);
return 0;
}
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()
On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak karthik@gmail.com wrote:
Extract two helper functions out of grab_single_ref(). Firstly,
new_refinfo() which is used to allocate memory for a new refinfo
structure and copy the objectname, refname and flag to it.
Secondly, match_name_as_path() which when given an array of patterns
and the refname checks if the refname matches any of the patterns
given while the pattern is a pathname, also while supporting wild
characters.
This is a preperatory patch for restructuring 'for-each-ref' and
eventually moving most of it to 'ref-filter' to provide the
functionality to similar commands via public API's.
Helped-by: Junio C Hamano gits...@pobox.com
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..b33e2de 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -837,6 +837,43 @@ struct grab_ref_cbdata {
};
/*
+ * Given a refname, return 1 if the refname matches with one of the patterns
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
+ */
+static int match_name_as_path(const char **pattern, const char *refname)
+{
+ int namelen = strlen(refname);
+ for (; *pattern; pattern++) {
+ const char *p = *pattern;
+ int plen = strlen(p);
+
+ if ((plen = namelen)
+ !strncmp(refname, p, plen)
+ (refname[plen] == '\0' ||
+refname[plen] == '/' ||
+p[plen-1] == '/'))
+ return 1;
+ if (!wildmatch(p, refname, WM_PATHNAME, NULL))
+ return 1;
+ }
+ return 0;
+}
+
+/* Allocate space for a new refinfo and copy the objectname and flag to it */
+static struct refinfo *new_refinfo(const char *refname,
+ const unsigned char *objectname,
+ int flag)
+{
+ struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+ ref-refname = xstrdup(refname);
+ hashcpy(ref-objectname, objectname);
+ ref-flag = flag;
+
+ return ref;
+}
+
+/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
@@ -851,40 +888,16 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
return 0;
}
- if (*cb-grab_pattern) {
- const char **pattern;
- int namelen = strlen(refname);
- for (pattern = cb-grab_pattern; *pattern; pattern++) {
- const char *p = *pattern;
- int plen = strlen(p);
-
- if ((plen = namelen)
- !strncmp(refname, p, plen)
- (refname[plen] == '\0' ||
-refname[plen] == '/' ||
-p[plen-1] == '/'))
- break;
- if (!wildmatch(p, refname, WM_PATHNAME, NULL))
- break;
- }
- if (!*pattern)
- return 0;
- }
+ if (*cb-grab_pattern !match_name_as_path(cb-grab_pattern,
refname))
+ return 0;
- /*
-* We do not open the object yet; sort may only need refname
-* to do its job and the resulting list may yet to be pruned
-* by maxcount logic.
-*/
Why did this comment get removed? Isn't it still meaningful to the
overall logic of this function?
- ref = xcalloc(1, sizeof(*ref));
- ref-refname = xstrdup(refname);
- hashcpy(ref-objectname, sha1);
- ref-flag = flag;
+ ref = new_refinfo(refname, sha1, flag);
cnt = cb-grab_cnt;
REALLOC_ARRAY(cb-grab_array, cnt + 1);
cb-grab_array[cnt++] = ref;
cb-grab_cnt = cnt;
+
Sneaking in whitespace change?
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Staging commits with visual diff tools?
On Tue, May 26, 2015 at 09:50:49PM +0100, John Lee wrote: Hi Does anybody have code to stage commits using a the visual diff/merge tools supported by git-difftool? Is there support in git itself somewhere, even? I'm looking for something functionally similar to git add -p Looking at the git-difftool source I can see how to write a command to do it, but wanted to check if it had already been implemented. Did I miss a way that already exists? Thanks John If you're interested in existing staging tools then I would highly suggest looking at git-cola. https://github.com/git-cola/git-cola git clone git://github.com/git-cola/git-cola.git git-cola is free software so it's pretty unique in many ways. I also wrote it, so that makes it that much more awesome ;-) cola has a very slick interactive staging editor, interactive rebase, and a whole lot more. I'm a g/vim user, so git-cola is finely tuned for keyboard usage. If we implement these feature in Git, we should consider providing the same workflows/hotkeys as cola. For staging, you can select lines, hit s, and they'll be staged/unstaged. ctrl-u reverts unstaged edits, etc. The '?' hotkey shows the help, etc. You can get extremely granular, moreso then add -p, which is why it exists. When cola's diff editor is not sufficient, you can also use it to drive difftool, but only in ways that you can on the command-line already. git-cola is written in Python(2+3) so it's easy to hack on, and quite portable. cheers, -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak karthik@gmail.com wrote:
Rename some of the functions and make them publically available.
s/publically/publicly/
This is a preparatory step for moving code from 'for-each-ref'
to 'ref-filter' to make meaningful, targeted services available to
other commands via public APIs.
Based-on-patch-by: Jeff King p...@peff.net
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f896e1c..8fed04b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -882,10 +882,10 @@ static struct ref_array_item *new_ref_array_item(const
char *refname,
}
/*
- * A call-back given to for_each_ref(). Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
Sneaking in whitespace change?
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1,
int flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
struct ref_filter_cbdata *ref_cbdata = cb_data;
struct ref_filter *filter = ref_cbdata-filter;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Make git-pull a builtin
Hi,
On Tue, May 19, 2015 at 3:21 AM, Junio C Hamano gits...@pobox.com wrote:
Paul Tan pyoka...@gmail.com writes:
This series rewrites git-pull.sh into a C builtin, thus improving its
performance and portability. It is part of my GSoC project to rewrite
git-pull
and git-am into builtins[2].
Earlier you were worried about 'git pull' being used in many tests
for the purpose of testing other parts of the system and not testing
'pull' itself. For a program as complex as 'git pull', you may want
to take the 'competing implementations' approach.
(1) write an empty cmd_pull() that relaunches git pull scripted
porcelain from the $GIT_EXEC_PATH with given parameters, and
wire all the necessary bits to git.c.
(2) enhance cmd_pull() a bit by bit, but keep something like this
if (getenv(GIT_USE_BUILTIN_PULL)) {
/* original run_command(git-pull.sh) code */
exit $?
}
... your C version ...
(3) add GIT_USE_BUILTIN_PULL=Yes; export GIT_USE_BUILTIN_PULL at
the beginning of t55?? test scripts (but not others that rely
on working pull and that are not interested in catching bugs in
pull).
(4) once cmd_pull() becomes fully operational, drop (3) and also the
conditional one you added in (2), and retire the environment
variable. Retire the git-pull.sh script to contrib/examples/
boneyard.
That way, you will always have a reference you can use throughout
the development.
Just a suggestion, not a requirement.
Okay, I'm trying this out in the next re-roll. I do agree that this
patch series should not touch anything in t/ at all.
One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and
leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2
targets to ./git-pull (they will clobber each other). For GNU Make,
the last defined target will win, so in this case it just happens that
git-pull.sh will win because the build targets for the shell scripts
are defined after the build targets for the builtins, so this works in
our favor I guess.
Regards,
Paul
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Make git-pull a builtin
On Sat, May 30, 2015 at 3:29 PM, Paul Tan pyoka...@gmail.com wrote: Hi, Okay, I'm trying this out in the next re-roll. I do agree that this patch series should not touch anything in t/ at all. One problem(?) is that putting builtins/pull.o in the BUILTIN_OBJS and leaving git-pull.sh in SCRIPT_SH in the Makefile will generate 2 targets to ./git-pull (they will clobber each other). For GNU Make, the last defined target will win, so in this case it just happens that git-pull.sh will win because the build targets for the shell scripts are defined after the build targets for the builtins, so this works in our favor I guess. Regards, Paul Just to add on, I just discovered that test-lib.sh unsets all GIT_* environment variables for repeatability, so the name GIT_USE_BUILTIN_PULL cannot be used. I'm tempted to just add a underscore just before the name (_GIT_USE_BUILTIN_PULL) to work around this, since it's just a temporary thing. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Diffing submodule does not yield complete logs for merge commits
On Fri, May 29, 2015 at 09:18:11PM -0500, Robert Dailey wrote: So I am working on trying to setup my environment (VM through Virtual Box) to do some testing on this. You all have encouraged me to try the mailing list review model. So I won't give up yet. I am not sure you need a VM or Linux environment. Of course it will be helpful in case your tests do no pass on Linux (which they sometimes do due to some differences between the OSes). But until we actually run into that problem I do not see anything wrong developing your change purely on Windows. Since it seems you are more familiar with that platform I would even encourage you to do so. That reduces the toolset friction which you might experience in a new environment. Even if you run into the problem, that your tests do not pass on Linux, we might be able to solve that on the list. Have you seen the github pull request - mailing list proxy thing[1]? If that helps you maybe you can test it and use it for your patch submission. I think nobody will be annoyed if we get some strange emails on the list during that testing phase since that might help more contributors to contribute. In the meantime I'd like to ask, do we even need to add an option for this? What if we just make `diff.submodule log` not use --first-parent? This seems like a backward compatible change in of itself. And it's simpler to implement. I can't think of a good justification to add more settings to an already hugely complex configuration scheme for such a minor difference in behavior. Thoughts? This behavior has been with --first-parent for a long time. Even though it seems like a minor change in my experience there will be complaints from people that have got used to it and will now get big differences. You never know how people use it. Since the extra value (full-log) (AFAIR) has reached a consensus on the list and it allows us to satisfy both long log and short log users I would prefer to go that route. Cheers Heiko [1] http://article.gmane.org/gmane.comp.version-control.git/269699 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] commit: fix ending newline for template files
@Eric, Junio Thank you a lot for feedback - should I post new set of patches as new thread with new cover letter, or reply to first mail in this thread? On Thu, May 28, 2015 at 4:29 PM, Eric Sunshine sunsh...@sunshineco.com wrote: Did you consider the alternate approach of handling newline processing immediately upon loading 'logfile' and 'template_file', rather than delaying processing until this point? Doing it that way would involve a bit of code repetition but might be easier to reason about since it would occur before possible interactions in following code (such as --signoff handling). Yes. I opted to place it in here, because newline was appended previously also in if (use_editor) block. But I agree, appending this newline after loading file will be cleaner - and code repetition may be avoided, if I'll separate file loading code into new function. On Thu, May 28, 2015 at 4:29 PM, Eric Sunshine sunsh...@sunshineco.com wrote: Moreover, it lacks justification and explanation of why you consider the cleanup unnecessary. History [1] indicates that its application to -F but not -t was intentional. That commit suggests, that cleanup was unintentional in one case, it says nothing about it being intentional for -F. Short story: currently cleanup on commit msg is performed many times (I am not sure if only 2 times or maybe more. I'll include more detailed analysis with second round of patches :) On Sat, May 30, 2015 at 12:25 AM, Eric Sunshine sunsh...@sunshineco.com wrote: I had a similar reaction. The one salient bit I picked up was that Patryk finds it aesthetically offensive[1] (...) Yes, that is exactly issue, that I initially wanted to solve. I didn't even notice, that my template had newline appended until I ran git-commit in gdb. Then I saw, that I can't actually test changes to newlines and rest followed, because I didn't want to leave code with more tests disabled. nano, vim, gedit (and other editors, I guess) append _and_hide_ \n before eof from user in default configuration. This newline appended by git before status is completely unexpected (and unwanted) behaviour IMHO. On Fri, May 29, 2015 at 4:17 PM, Junio C Hamano gits...@pobox.com wrote: in other words, if your template ends with an incomplete line and it causes you trouble, then do not do that!. 1) But problem occurs, when template ends with complete line. To make it disappear, user needs to somehow remove trailing newline from his file. In vim it involves switching to non-default binary mode, in nano or gedit it's impossible. Anwer use emacs would be a bit disrespectful towards end user ;) 2) That commit addresses different issue - when user intentionally left whitespace in template file, then commit should not clean it up, because it might've beed a form to be filled. 3) Well, the exact same logic can be applied to logfile - it does not explain why logfiles and template files should be treated differently in this regard. In fact, when looking at 8b1ae67, I think that lack of this cleanup for logfiles might be an unintended ommision. After another look at that commit - included test doesn't actually verify implemented change (commit msg is stripped and commit_msg_is doesn't verify newlines anyway). On Sat, May 30, 2015 at 12:25 AM, Eric Sunshine sunsh...@sunshineco.com wrote: If the user specified with the --cleanup option not to clean-up the result coming back from the editor, then the commented material needs to be removed in the editor by the user *anyway*. Why? Is it not ok to leave lines starting with hash in commit object? --cleanup=whitespace|verbatim suggests, that it's a valid usecase. -- | ← Ceci n'est pas une pipe Patryk Obara -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clean: remove unused variable buf
2015-05-20 6:13 GMT+08:00 René Scharfe l@web.de:
It had never been used.
Confirmed. The unused buf should be removed.
Signed-off-by: Rene Scharfe l@web.de
---
builtin/clean.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 98c103f..a9eb772 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -314,7 +314,6 @@ static void print_highlight_menu_stuff(struct menu_stuff
*stuff, int **chosen)
{
struct string_list menu_list = STRING_LIST_INIT_DUP;
struct strbuf menu = STRBUF_INIT;
- struct strbuf buf = STRBUF_INIT;
struct menu_item *menu_item;
struct string_list_item *string_list_item;
int i;
@@ -363,7 +362,6 @@ static void print_highlight_menu_stuff(struct menu_stuff
*stuff, int **chosen)
pretty_print_menus(menu_list);
strbuf_release(menu);
- strbuf_release(buf);
string_list_clear(menu_list, 0);
}
--
2.4.1
--
Jiang Xin
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] completion: suggest sequencer commands for revert
Signed-off-by: Thomas Braun thomas.br...@virtuell-zuhause.de
---
contrib/completion/git-completion.bash | 8
1 file changed, 8 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index f6e5bf6..486c61b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -868,6 +868,12 @@ __git_complete_sequencer ()
return 0
fi
;;
+ revert)
+ if [ -f $dir/REVERT_HEAD ]; then
+ __gitcomp --continue --quit --abort
+ return 0
+ fi
+ ;;
rebase)
if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ];
then
__gitcomp --continue --skip --abort
@@ -2300,6 +2306,8 @@ _git_reset ()
_git_revert ()
{
+ __git_complete_sequencer revert return
+
case $cur in
--*)
__gitcomp --edit --mainline --no-edit --no-commit --signoff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] completion: sequencer commands
Ramkumar Ramachandra wrote:
Junio C Hamano wrote:
contrib/completion/git-completion.bash | 5 +
1 file changed, 5 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index bfc74e9..3c00acd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2282,6 +2282,11 @@ _git_reset ()
_git_revert ()
{
+ local dir=$(__gitdir)
+ if [ -f $dir/REVERT_HEAD ]; then
+ __gitcomp --continue --quit --abort
+ return
+ fi
case $cur in
--*)
__gitcomp --edit --mainline --no-edit --no-commit
--signoff
This corresponds exactly to what we do for git-cherry-pick:
local dir=$(__gitdir)
if [ -f $dir/CHERRY_PICK_HEAD ]; then
__gitcomp --continue --quit --abort
return
fi
Perhaps _git_revert() and _git_cherry_pick() should call into the same
function with different arguments.
Good idea.
I created a new function __git_complete_sequencer which is now used to complete
all commands with active sequencer.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] completion: Add sequencer function
Signed-off-by: Thomas Braun thomas.br...@virtuell-zuhause.de
---
contrib/completion/git-completion.bash | 48 +++---
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index bfc74e9..f6e5bf6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -851,15 +851,40 @@ __git_count_arguments ()
printf %d $c
}
+__git_complete_sequencer ()
+{
+ local dir=$(__gitdir)
+
+ case $1 in
+ am)
+ if [ -d $dir/rebase-apply ]; then
+ __gitcomp --skip --continue --resolved --abort
+ return 0
+ fi
+ ;;
+ cherry-pick)
+ if [ -f $dir/CHERRY_PICK_HEAD ]; then
+ __gitcomp --continue --quit --abort
+ return 0
+ fi
+ ;;
+ rebase)
+ if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ];
then
+ __gitcomp --continue --skip --abort
+ return 0
+ fi
+ ;;
+ esac
+
+ return 1
+}
+
__git_whitespacelist=nowarn warn error error-all fix
_git_am ()
{
- local dir=$(__gitdir)
- if [ -d $dir/rebase-apply ]; then
- __gitcomp --skip --continue --resolved --abort
- return
- fi
+ __git_complete_sequencer am return
+
case $cur in
--whitespace=*)
__gitcomp $__git_whitespacelist ${cur##--whitespace=}
@@ -1044,11 +1069,8 @@ _git_cherry ()
_git_cherry_pick ()
{
- local dir=$(__gitdir)
- if [ -f $dir/CHERRY_PICK_HEAD ]; then
- __gitcomp --continue --quit --abort
- return
- fi
+ __git_complete_sequencer cherry-pick return
+
case $cur in
--*)
__gitcomp --edit --no-commit --signoff --strategy= --mainline
@@ -1666,11 +1688,7 @@ _git_push ()
_git_rebase ()
{
- local dir=$(__gitdir)
- if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ]; then
- __gitcomp --continue --skip --abort
- return
- fi
+ __git_complete_sequencer rebase return
__git_complete_strategy return
case $cur in
--whitespace=*)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] completion: Add sequencer function
Signed-off-by: Thomas Braun thomas.br...@virtuell-zuhause.de
---
contrib/completion/git-completion.bash | 48 +++---
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index bfc74e9..f6e5bf6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -851,15 +851,40 @@ __git_count_arguments ()
printf %d $c
}
+__git_complete_sequencer ()
+{
+ local dir=$(__gitdir)
+
+ case $1 in
+ am)
+ if [ -d $dir/rebase-apply ]; then
+ __gitcomp --skip --continue --resolved --abort
+ return 0
+ fi
+ ;;
+ cherry-pick)
+ if [ -f $dir/CHERRY_PICK_HEAD ]; then
+ __gitcomp --continue --quit --abort
+ return 0
+ fi
+ ;;
+ rebase)
+ if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ];
then
+ __gitcomp --continue --skip --abort
+ return 0
+ fi
+ ;;
+ esac
+
+ return 1
+}
+
__git_whitespacelist=nowarn warn error error-all fix
_git_am ()
{
- local dir=$(__gitdir)
- if [ -d $dir/rebase-apply ]; then
- __gitcomp --skip --continue --resolved --abort
- return
- fi
+ __git_complete_sequencer am return
+
case $cur in
--whitespace=*)
__gitcomp $__git_whitespacelist ${cur##--whitespace=}
@@ -1044,11 +1069,8 @@ _git_cherry ()
_git_cherry_pick ()
{
- local dir=$(__gitdir)
- if [ -f $dir/CHERRY_PICK_HEAD ]; then
- __gitcomp --continue --quit --abort
- return
- fi
+ __git_complete_sequencer cherry-pick return
+
case $cur in
--*)
__gitcomp --edit --no-commit --signoff --strategy= --mainline
@@ -1666,11 +1688,7 @@ _git_push ()
_git_rebase ()
{
- local dir=$(__gitdir)
- if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ]; then
- __gitcomp --continue --skip --abort
- return
- fi
+ __git_complete_sequencer rebase return
__git_complete_strategy return
case $cur in
--whitespace=*)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
