* [PATCH 4/6] Implement parse_pathspec()
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>
This function is the same as get_pathspec() except that it produces
struct pathspec directly.
no_prefix code path is necessary because init_pathspec() can be called
without get_pathspec_old() in "diff --no-index" case. Without this
exception, prefix_path() will be eventually called and die because
there is not worktree.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
cache.h | 5 ++++
dir.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++----------------
setup.c | 4 +-
3 files changed, 68 insertions(+), 23 deletions(-)
diff --git a/cache.h b/cache.h
index 17df06f..719d4a3 100644
--- a/cache.h
+++ b/cache.h
@@ -443,6 +443,9 @@ extern void set_git_work_tree(const char *tree);
#define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
+struct pathspec_item;
+extern const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+ int prefixlen, const char *elt);
extern const char **get_pathspec(const char *prefix, const char **pathspec);
extern const char *pathspec_prefix(const char *prefix, const char **pathspec);
extern void setup_work_tree(void);
@@ -554,6 +557,8 @@ struct pathspec {
};
extern int init_pathspec(struct pathspec *, const char **);
+extern int parse_pathspec(struct pathspec *pathspec, const char *prefix,
+ int argc, const char **argv);
extern void free_pathspec(struct pathspec *);
extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
diff --git a/dir.c b/dir.c
index 6c82615..d38af0f 100644
--- a/dir.c
+++ b/dir.c
@@ -18,6 +18,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, in
int check_only, const struct path_simplify *simplify);
static int get_dtype(struct dirent *de, const char *path, int len);
+static const char *no_prefix = "We do not want outside repository check.";
+
/* helper string functions with support for the ignore_case flag */
int strcmp_icase(const char *a, const char *b)
{
@@ -1252,34 +1254,62 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
return strcmp(a->match, b->match);
}
-int init_pathspec(struct pathspec *pathspec, const char **paths)
+extern const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+ int prefixlen, const char *elt);
+int parse_pathspec(struct pathspec *pathspec, const char *prefix,
+ int argc, const char **argv)
{
- const char **p = paths;
- int i;
+ struct pathspec_item *pitem;
+ const char **dst;
+ int prefixlen;
memset(pathspec, 0, sizeof(*pathspec));
- if (!p)
- return 0;
- while (*p)
- p++;
- pathspec->raw = paths;
- pathspec->nr = p - paths;
- pathspec->magic = PATHSPEC_NOGLOB;
- if (!pathspec->nr)
+
+ if (argc == -1) {
+ argc = 0;
+ for (dst = argv; *dst; dst++)
+ argc++;
+ }
+
+ if ((!prefix || prefix == no_prefix) && !argc)
return 0;
- pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
- memset(pathspec->items, 0, sizeof(struct pathspec_item)*pathspec->nr);
- for (i = 0; i < pathspec->nr; i++) {
- struct pathspec_item *item = pathspec->items+i;
- const char *path = paths[i];
+ if (!argc) {
+ static const char *spec[2];
+ spec[0] = prefix;
+ spec[1] = NULL;
+ init_pathspec(pathspec, spec);
+ pathspec->items[0].plain_len = pathspec->items[0].len;
+ return 0;
+ }
- item->match = path;
- item->len = strlen(path);
- if (no_wildcard(path))
- item->magic |= PATHSPEC_NOGLOB;
+ prefixlen = prefix && prefix != no_prefix ? strlen(prefix) : 0;
+ pathspec->nr = argc; /* be optimistic, lower it later if necessary */
+ pathspec->items = xmalloc(sizeof(struct pathspec_item) * argc);
+ pathspec->raw = argv;
+ pathspec->magic = PATHSPEC_NOGLOB;
+ pitem = pathspec->items;
+ dst = argv;
+
+ while (*argv) {
+ if (prefix == no_prefix) {
+ memset(pitem, 0, sizeof(*pitem));
+ pitem->match = *argv;
+ pitem->len = strlen(pitem->match);
+ }
+ else
+ prefix_pathspec(pitem, prefix, prefixlen, *argv);
+ *dst = *argv++;
+ if (pitem->match && pitem->len) {
+ if (no_wildcard(pitem->match + pitem->plain_len))
+ pitem->magic |= PATHSPEC_NOGLOB;
+ else
+ pathspec->magic &= ~PATHSPEC_NOGLOB;
+ pitem++;
+ dst++;
+ }
else
- pathspec->magic &= ~PATHSPEC_NOGLOB;
+ pathspec->nr--;
}
qsort(pathspec->items, pathspec->nr,
@@ -1288,8 +1318,18 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
return 0;
}
+int init_pathspec(struct pathspec *pathspec, const char **paths)
+{
+ const char **p = paths;
+ while (p && *p)
+ p++;
+ return parse_pathspec(pathspec, no_prefix, p - paths, paths);
+}
+
void free_pathspec(struct pathspec *pathspec)
{
+ /* memory leak: pathspec_item->match likely be xstrdup'd by
+ prefix_pathspec */
free(pathspec->items);
pathspec->items = NULL;
}
diff --git a/setup.c b/setup.c
index 8f1c2c0..b074210 100644
--- a/setup.c
+++ b/setup.c
@@ -126,8 +126,8 @@ static struct pathspec_magic {
* the prefix part must always match literally, and a single stupid
* string cannot express such a case.
*/
-static const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
- int prefixlen, const char *elt)
+const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+ int prefixlen, const char *elt)
{
unsigned magic = 0;
const char *copyfrom = elt;
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH 3/6] Convert prefix_pathspec() to produce struct pathspec_item
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>
New field plain_len is used to mark the first part of 'match' where no
magic can be applied. It's not used though. tree_entry_interesting()
should learn to utilize it.
Now we can show get_pathspec() what magic a pathspec has. Make sure
only certain magic is accepted because more will come in future and
not all of them can be converted to plain string like
PATHSPEC_FROMTOP.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
match_pathspec_depth() should also check for unsupported magic..
cache.h | 1 +
setup.c | 22 +++++++++++++++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index fe46f1e..17df06f 100644
--- a/cache.h
+++ b/cache.h
@@ -548,6 +548,7 @@ struct pathspec {
struct pathspec_item {
const char *match;
int len;
+ int plain_len; /* match[0..plain_len-1] does not have any kind of magic*/
unsigned magic;
} *items;
};
diff --git a/setup.c b/setup.c
index 35db910..8f1c2c0 100644
--- a/setup.c
+++ b/setup.c
@@ -126,7 +126,8 @@ static struct pathspec_magic {
* the prefix part must always match literally, and a single stupid
* string cannot express such a case.
*/
-static const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
+static const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+ int prefixlen, const char *elt)
{
unsigned magic = 0;
const char *copyfrom = elt;
@@ -181,10 +182,17 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char
copyfrom++;
}
+ memset(item, 0, sizeof(*item));
+ item->magic = magic;
+
if (magic & PATHSPEC_FROMTOP)
- return xstrdup(copyfrom);
- else
- return prefix_path(prefix, prefixlen, copyfrom);
+ item->match = xstrdup(copyfrom);
+ else {
+ item->match = prefix_path(prefix, prefixlen, copyfrom);
+ item->plain_len = prefixlen;
+ }
+ item->len = strlen(item->match);
+ return 0;
}
const char **get_pathspec(const char *prefix, const char **pathspec)
@@ -208,7 +216,11 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
dst = pathspec;
prefixlen = prefix ? strlen(prefix) : 0;
while (*src) {
- *(dst++) = prefix_pathspec(prefix, prefixlen, *src);
+ struct pathspec_item item;
+ prefix_pathspec(&item, prefix, prefixlen, *src);
+ if (item.magic & ~PATHSPEC_FROMTOP)
+ die("Unsupported magic pathspec %s", *src);
+ *(dst++) = item.match;
src++;
}
*dst = NULL;
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH 2/6] Replace has_wildcard with PATHSPEC_NOGLOB
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>
Note though "noglob" magic is not implemented yet, only its definition
for internal use.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/ls-files.c | 2 +-
builtin/ls-tree.c | 4 ++--
cache.h | 22 ++++++++++++++++++++--
dir.c | 11 +++++++----
setup.c | 17 -----------------
tree-walk.c | 7 +++----
6 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index e8a800d..e687157 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -326,7 +326,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
matchbuf[0] = prefix;
matchbuf[1] = NULL;
init_pathspec(&pathspec, matchbuf);
- pathspec.items[0].use_wildcard = 0;
+ pathspec.items[0].magic = PATHSPEC_NOGLOB;
} else
init_pathspec(&pathspec, NULL);
if (read_tree(tree, 1, &pathspec))
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 6b666e1..f0fa7dd 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -168,8 +168,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
for (i = 0; i < pathspec.nr; i++)
- pathspec.items[i].use_wildcard = 0;
- pathspec.has_wildcard = 0;
+ pathspec.items[i].magic = PATHSPEC_NOGLOB;
+ pathspec.magic |= PATHSPEC_NOGLOB;
tree = parse_tree_indirect(sha1);
if (!tree)
die("not a tree object");
diff --git a/cache.h b/cache.h
index 82e12c8..fe46f1e 100644
--- a/cache.h
+++ b/cache.h
@@ -521,16 +521,34 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
+/*
+ * Magic pathspec
+ *
+ * NEEDSWORK: These need to be moved to dir.h or even to a new
+ * pathspec.h when we restructure get_pathspec() users to use the
+ * "struct pathspec" interface.
+ *
+ * Possible future magic semantics include stuff like:
+ *
+ * { PATHSPEC_NOGLOB, '!', "noglob" },
+ * { PATHSPEC_ICASE, '\0', "icase" },
+ * { PATHSPEC_RECURSIVE, '*', "recursive" },
+ * { PATHSPEC_REGEXP, '\0', "regexp" },
+ *
+ */
+#define PATHSPEC_FROMTOP (1<<0)
+#define PATHSPEC_NOGLOB (1<<1)
+
struct pathspec {
const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
int nr;
- unsigned int has_wildcard:1;
+ unsigned magic;
unsigned int recursive:1;
int max_depth;
struct pathspec_item {
const char *match;
int len;
- unsigned int use_wildcard:1;
+ unsigned magic;
} *items;
};
diff --git a/dir.c b/dir.c
index 08281d2..6c82615 100644
--- a/dir.c
+++ b/dir.c
@@ -230,7 +230,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
return MATCHED_RECURSIVELY;
}
- if (item->use_wildcard && !fnmatch(match, name, 0))
+ if (!(item->magic & PATHSPEC_NOGLOB) && !fnmatch(match, name, 0))
return MATCHED_FNMATCH;
return 0;
@@ -1264,19 +1264,22 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
p++;
pathspec->raw = paths;
pathspec->nr = p - paths;
+ pathspec->magic = PATHSPEC_NOGLOB;
if (!pathspec->nr)
return 0;
pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
+ memset(pathspec->items, 0, sizeof(struct pathspec_item)*pathspec->nr);
for (i = 0; i < pathspec->nr; i++) {
struct pathspec_item *item = pathspec->items+i;
const char *path = paths[i];
item->match = path;
item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
- if (item->use_wildcard)
- pathspec->has_wildcard = 1;
+ if (no_wildcard(path))
+ item->magic |= PATHSPEC_NOGLOB;
+ else
+ pathspec->magic &= ~PATHSPEC_NOGLOB;
}
qsort(pathspec->items, pathspec->nr,
diff --git a/setup.c b/setup.c
index 08f605b..35db910 100644
--- a/setup.c
+++ b/setup.c
@@ -105,23 +105,6 @@ void verify_non_filename(const char *prefix, const char *arg)
"Use '--' to separate filenames from revisions", arg);
}
-/*
- * Magic pathspec
- *
- * NEEDSWORK: These need to be moved to dir.h or even to a new
- * pathspec.h when we restructure get_pathspec() users to use the
- * "struct pathspec" interface.
- *
- * Possible future magic semantics include stuff like:
- *
- * { PATHSPEC_NOGLOB, '!', "noglob" },
- * { PATHSPEC_ICASE, '\0', "icase" },
- * { PATHSPEC_RECURSIVE, '*', "recursive" },
- * { PATHSPEC_REGEXP, '\0', "regexp" },
- *
- */
-#define PATHSPEC_FROMTOP (1<<0)
-
static struct pathspec_magic {
unsigned bit;
char mnemonic; /* this cannot be ':'! */
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..db07fd6 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -586,7 +586,7 @@ int tree_entry_interesting(const struct name_entry *entry,
{
int i;
int pathlen, baselen = base->len - base_offset;
- int never_interesting = ps->has_wildcard ? 0 : -1;
+ int never_interesting = ps->magic & PATHSPEC_NOGLOB ? -1 : 0;
if (!ps->nr) {
if (!ps->recursive || ps->max_depth == -1)
@@ -625,7 +625,7 @@ int tree_entry_interesting(const struct name_entry *entry,
&never_interesting))
return 1;
- if (ps->items[i].use_wildcard) {
+ if (!(ps->items[i].magic & PATHSPEC_NOGLOB)) {
if (!fnmatch(match + baselen, entry->path, 0))
return 1;
@@ -636,12 +636,11 @@ int tree_entry_interesting(const struct name_entry *entry,
if (ps->recursive && S_ISDIR(entry->mode))
return 1;
}
-
continue;
}
match_wildcards:
- if (!ps->items[i].use_wildcard)
+ if (ps->items[i].magic & PATHSPEC_NOGLOB)
continue;
/*
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH 1/6] Recognize magic pathspec as filenames
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
.. so that "git log :/" works, not so sure this is correct though
setup.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/setup.c b/setup.c
index 27c1d47..08f605b 100644
--- a/setup.c
+++ b/setup.c
@@ -58,15 +58,8 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg)
unsigned char sha1[20];
unsigned mode;
- /*
- * Saying "'(icase)foo' does not exist in the index" when the
- * user gave us ":(icase)foo" is just stupid. A magic pathspec
- * begins with a colon and is followed by a non-alnum; do not
- * let get_sha1_with_mode_1(only_to_die=1) to even trigger.
- */
- if (!(arg[0] == ':' && !isalnum(arg[1])))
- /* try a detailed diagnostic ... */
- get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix);
+ /* try a detailed diagnostic ... */
+ get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix);
/* ... or fall back the most general message. */
die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
@@ -85,6 +78,11 @@ void verify_filename(const char *prefix, const char *arg)
{
if (*arg == '-')
die("bad flag '%s' used after filename", arg);
+
+ /* If it's magic pathspec, just assume it's file name */
+ if (arg[0] == ':' && is_pathspec_magic(arg[1]))
+ return;
+
if (check_filename(prefix, arg))
return;
die_verify_filename(prefix, arg);
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH 0/6] Negation magic pathspec
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
After the last round toying with .gitignore mechanism as a way to
exclude paths, I have finally got back to the negative pathspec.
I'm still struggling with read_directory() rewrite so that struct
pathspec can be used throughout git, but now realized we can at least
enable magic for certain commands and die() on those that don't.
This may help move magic pathspec patches forward.
The nice thing about this series is that negative pathspec patch is
small and simple, much less headache to review than the previous
version (and as a consequence, not as powerful).
So here it is to gather comments whether we should go this way. Very
much WIP, I have not even run "make test".
Nguyễn Thái Ngọc Duy (6):
Recognize magic pathspec as filenames
Replace has_wildcard with PATHSPEC_NOGLOB
Convert prefix_pathspec() to produce struct pathspec_item
Implement parse_pathspec()
Convert simple init_pathspec() cases to parse_pathspec()
Implement negative pathspec
Documentation/glossary-content.txt | 8 ++--
builtin/grep.c | 4 +-
builtin/ls-files.c | 2 +-
builtin/ls-tree.c | 6 +-
builtin/reset.c | 2 +-
cache.h | 29 +++++++++++-
dir.c | 85 +++++++++++++++++++++++++++--------
revision.c | 9 ++--
setup.c | 56 +++++++++++-------------
tree-walk.c | 44 ++++++++++++++++---
10 files changed, 169 insertions(+), 76 deletions(-)
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply
* Re: [RFC] teach --edit to git rebase
From: Jean Privat @ 2011-10-11 22:36 UTC (permalink / raw)
To: git
In-Reply-To: <CAMQw0oOBEjW3yS2+wcktXDuEuUiHKjfbK2qDzKvBOiwxo7Zkow@mail.gmail.com>
> An other alternative is to use a simple git alias for myself (and do
> not bother the git community) but I do not know how to automatize the
> command 'git rebase --interactive' I suppose I need more complex
> infrastructure in the existing 'git rebase' (Maybe I did not look
> enough and there is a way to do it with a git alias).
Hum. I just found that I can do something like :
[alias]
edit=!GIT_EDITOR='sed -i -e 1s/pick/edit/ --' git rebase -i
-no-autosquash $1^
The main thing that bothers me is that after the rebase, the head is
detached (why?)
The other thing is that the error messages for invalid usages are far
for perfect
$ git edit toto
fatal: Needed a single revision
invalid upstream toto^
$ git edit --root toto
error: unknown option `root^'
usage: git rebase [-i] [options] [--onto <newbase>] [<upstream>] [<branch>]
[...]
-Jean
^ permalink raw reply
* Re: Symmetric of rebase / more intelligent cherry-pick
From: Jonathan Nieder @ 2011-10-11 22:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Lionel Elie Mamane, Christian Couder, Michael J Gruber
In-Reply-To: <7vty7fdxql.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> We actually have half of that filtering in "--cherry-pick" that was
> introduced in d7a17ca (git-log --cherry-pick A...B, 2007-04-09).
>
> Perhaps the recent cherry-pick that allows multiple commits to be picked
> should use that option (i.e. a "log --cherry-pick --right-only ..@{u}"
> equivalent) when coming up with which commits to apply?
No UI ideas from me, but I have a script like this sitting in $HOME/bin:
base=$1 topic=$2
git rev-list $base..HEAD |
git diff-tree -s --pretty=raw --stdin |
sed -ne 's/(cherry picked from commit \([0-9a-f]*\))/\1/p' |
xargs git cherry-pick -x -s \
--cherry-pick --right-only --no-merges HEAD...$topic \
--not $base
^ permalink raw reply
* Re: [RESEND PATCH v3] Configurable hyperlinking in gitk
From: Junio C Hamano @ 2011-10-11 22:13 UTC (permalink / raw)
To: Jeff Epler, Paul Mackerras, Jakub Narebski
Cc: git, Marc Branchaud, Chris Packham
In-Reply-To: <20111011183722.GA26646@unpythonic.net>
Jeff Epler <jepler@unpythonic.net> writes:
> I'm aware of no problems with this patch, and a number of people have
> commented that it is useful to them.
Hmmm, "didn't generate any discussion" does not mesh very well with "a
number of people are happy". Which one should I trust?
> * Documented that these are POSIX EREs; hopefully that's OK. I see
> in CodingGuidelines that in git itself "a subset of BREs" are used,
> so maybe even this is too much power. And hopefully tcl's
> re_syntax really is close enough to an ERE superset that this isn't
> a terrible lie about the initial implementation either.
I think it is better to be honest and say these are fed to the native
regexp engine of Tcl somewhere in the documentation.
Declaring "these are POSIX EREs" invites a user to expect they truly
are. When the pattern the user wrote triggers a strange Tcl extension to
cause unexpected things to match, the documentation needs to help the user
to understand why. I understand the longer-term wish to reuse these in
gitweb and elsewhere, but it becomes even more important that it is
clearly documented that these "regexp" are fed to native regexp engines of
Tcl and Perl depending on the program that they are used in. Unless the
documentation spells it out, the user will not be able to decide how to
work the implementation around, avoiding constructs that would behave
differently between Tcl and Perl.
Doesn't tcl have/use pcre these days, by the way? If we envision that this
will be shared with gitweb, perhaps using that might be a better option to
reduce potential confusion.
> * Added a Signed-Off-By, since I've had a number of positive feedbacks
> and the only problems I've heard of (since patch v2) are the ones
> related to 'eval' in git-web--browse.
By the way, "This patch is good" does not have anything to do with signing
off a patch.
Paul wanted to keep gitk sources separately available from the rest of the
git. After all, that is how gitk project started. Even after 5569bf9 (Do a
cross-project merge of Paul Mackerras' gitk visualizer, 2005-06-22), we
kept it so that git://git.kernel.org/pub/scm/gitk/gitk was the primary
project to make changes to gitk, and git.git pulled from it (it is an
assymmetric pull, as gitk cannot pull from git without contaminating its
history with the changes to the rest of git).
I do not know how motivated Paul is to keep gitk part separated in its own
project these days. I do not think the /pub/scm/gitk/gitk repository has
been re-populated yet. Assuming that it will eventually come back on-line,
could you send the gitk part of this change to Paul (i.e. the diff header
of your patch should read "diff --git a/gitk b/gitk") and another separate
patch to Documentation/ part?
Paul, if you are orphaning gitk, I do _not_ mind start taking patches that
touch gitk myself directly into git tree.
But I would still need reviewers who are motivated and interested in
enhancing and maintaining gitk.
> +linkify.<name>.regexp::
> + Specify a regular expression in the POSIX Extended Regular Expression
> + syntax defining a class of strings to automatically convert to
> + hyperlinks. This regular expression many not span multiple lines.
> + You must also specify 'linkify.<name>.subst'.
Saying "You must ..." without explicitly saying "why" is a bad style. If
the reader already _knows_ the .regexp is used to supply captured
substring to the corresponding .subst, then it is obvious that whenever
you have .regexp you need a matching .subst, but that is not even
explained here.
How about this?
A string that matches this regexp is converted to a hyperlink
using the value of corresponding `linkify.<name>.subst` variable.
The regular expression is passed to the regexp engine of Tcl (in
gitk) or Perl (in gitweb).
> +linkify.<name>.subst::
> + Specify a substitution that results in the target URL for the
> + related regular expression. Back-references like '\1' refer
> + to capturing groups in the associated regular expression.
> + You must also specify 'linkify.<name>.regexp'.
Likewise.
A string matched the value of the corresponding
`linkify.<name>.regexp` variable is rewritten to this URL. The
value of this variable can contain back-references like `\1` to
refer to capturing groups in the associated regular expression.
^ permalink raw reply
* Re: [PATCH] Fix is_gitfile() for files larger than PATH_MAX
From: Phil Hord @ 2011-10-11 22:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <CABURp0ru7aCW_oUZO8eaFpau5DAHDgwWLqHSL1QMjbUDkqrANg@mail.gmail.com>
On Tue, Oct 11, 2011 at 4:58 PM, Phil Hord <phil.hord@gmail.com> wrote:
> I've caught this and have it in
> a re-roll, but I got mired up and haven't submitted it again. I'll
> try to do so today.
On 2nd thought, my other changes are cleanup and not so much a re-roll
of this (except for a couple of code style edits pointed out by Junio,
but I guess those can slide).
I'll send the cleanups and generalities in another patch.
Thanks for the catch, Johannes.
Phil
^ permalink raw reply
* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-11 21:31 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Michael Haggerty, git, cmn, A Large Angry SCM,
Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <20111011203903.GA23069@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But in the code, it is spelled RENAMED-REF (with a dash). And as far as
> I can tell, does not actually create a reflog. And it's not documented
> anywhere, so I suspect nobody is using it. Maybe it is worth switching
> that name.
Or even better get rid of it?
>> - dwim_ref() can be fed "refs/heads/master" and is expected to dwim it to
>> the master branch.
>
> It looks like your code will allow any subdirectory. I had thought to
> limit it to "refs/". Otherwise, my "config" example could be
> "objects/pack", or "lost-found/commits", "remotes/foo", or something.
> Obviously the longer the name, the smaller the possibility of an
> accidental collision. But I couldn't think of any other subdirectory
> into which refs should go.
I wanted to start as loose as possible to avoid negatively impacting
existing users, later to tighten. As fsck and friends never look outside
of refs/, I think the prefix refs/ is a reasonable restriction that is
safe.
^ permalink raw reply
* Re: Symmetric of rebase / more intelligent cherry-pick
From: Junio C Hamano @ 2011-10-11 21:28 UTC (permalink / raw)
To: git; +Cc: Lionel Elie Mamane
In-Reply-To: <7v8vorfhhu.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Lionel Elie Mamane <lionel@mamane.lu> writes:
>
>> git cherry-pick ..UPSTREAM
>> *nearly* does what I want, except that it lacks rebase's intelligence
>> of skipping commits that do the same textual changes as a commit
>> already in the current branch.
>
> I think in the longer term "--ignore-if-in-upstream" that is known only to
> format-patch, which is the true source the intelligence of rebase you
> observed comes from, should be factored out into a helper function that
> can be used to filter output from get_revision() in other commands, or
> perhaps get_revision() itself might want to learn it.
We actually have half of that filtering in "--cherry-pick" that was
introduced in d7a17ca (git-log --cherry-pick A...B, 2007-04-09).
Perhaps the recent cherry-pick that allows multiple commits to be picked
should use that option (i.e. a "log --cherry-pick --right-only ..@{u}"
equivalent) when coming up with which commits to apply?
^ permalink raw reply
* [RFC] teach --edit to git rebase
From: Jean Privat @ 2011-10-11 21:21 UTC (permalink / raw)
To: git
The idea of this patch is to allow a simple edition of a buggy commit
in the history.
## Motivation
The motivation behind the option is that sometime I have to do a
simple fixup of a previous commit.
Usually the way to do it is just to commit the fix on the top of the
branch (git commit --fixup) then doing a 'git rebase --autosquash'.
However, the way is often not optimal if the context of commit on the
top of the branch is different from the context of the buggy commit,
thus the rebase with a fixup will lead to a conflict when git will
apply the fixup patch to the buggy commit (and a second conflict
later).
An other way is to do a 'git rebase --interactive' with an edit in the
todo list (instead of a pick). This is what I propose to simplify.
## Proposal
My idea is to add a --edit option to 'git rebase' in order to
automatize the 'git rebase --interactive' workflow.
Currently:
$ git rebase -i commit-to-fix^
# replace the first 'pick' with 'edit' then save and quit
$ hack hack hack...
$ git commit --amend # or whatever you want to do like split commit
$ git rebase --continue # and resolve possible conflicts
With the --edit option:
$ git rebase --edit commit_to_fix # note: no caret, no editor ! yeah !
$ hack hack hack...
$ git commit --amend # or what ever like split commit
$ git rebase --continue # and resolve possible conflicts
Note that for a more complex history modification, a standard git
rebase with reordering, squashing and stuff the way to go is a good
old git rebase --interactive.
## Implementation proposal
Yes I know "show me the code" but because I am lazy I prefer ask 1-if
the proposal makes sense, and 2-if the following way of doing it makes
sense.
The idea is to extend git-rebase and git-rebase-interactive.
* detect and check the option --edit in git-rebase
* automatically prepare the todolist in git-rebase-interactive without
launching the editor.
## Alternative proposals
A weak point of the proposal it that it is a new option to a
overloaded git command (git rebase). In fact is is even a new synopsis
to git rebase since the --edit option will be incompatible with
--interactive (it is an automatic interactive) and with --onto (since
there is no real point to ``move'' the base if you want to fixup a
single commit).
A counter proposal could be to not extend the command 'git rebase' but
add a new git command (for instance 'git edit buggy_commit') but since
the edit may[1] lead to conflicts the user has to do some 'git edit
--continue' to finish the editing (or 'git edit --abort' if bored). It
will also require to adapt a lot of tinny things because hint
messages, error messages, and prompts will talk about 'rebase' and not
'edit'.
[1] In fact, it is probable that the *may* is in fact a *will* since
if no conflict arise, it is likable that a simple 'commit --fixup'
(followed by a later 'git rebase --autosquash') will just work and be
simpler.
An other alternative is to use a simple git alias for myself (and do
not bother the git community) but I do not know how to automatize the
command 'git rebase --interactive' I suppose I need more complex
infrastructure in the existing 'git rebase' (Maybe I did not look
enough and there is a way to do it with a git alias).
A last alternative is do do nothing. What I propose is just a way to
1-do not need a caret ('git rebase --edit commit' instead of 'git
rebase --interactive commit^') and 2-avoid launching the editor.
Therefore, maybe the itch do not really need a patch.
-- Jean Privat
^ permalink raw reply
* Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Peter Oberndorfer @ 2011-10-11 21:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vipnvfk70.fsf@alter.siamese.dyndns.org>
On Dienstag, 11. Oktober 2011, Junio C Hamano wrote:
> Peter Oberndorfer <kumbayo84@arcor.de> writes:
>
> > Using $GIT_EDITOR or core.editor config var for this is not possible
> > since it is also used to start the commit message editor for reword action.
>
> Your tool _could_ be smart about this issue and inspect the contents to
> launch a real editor when it is fed a material not for sequencing, but
> that feels hacky.
I already tried this, but my first version did not redirect stdin/stdout
so vi stayed in background and the whole thing just hung.
I did not try further because i assumed more problems would appear
when redirecting stdin/stdout...
> > * GIT_EDITOR env var is not honored anymore after this change.
>
> Care to explain? "git var" knows magic about a few variables like
> GIT_EDITOR and GIT_PAGER.
>
> $ git config core.editor vim
> $ GIT_EDITOR=vi EDITOR=emacs git var GIT_EDITOR
> vi
> $ unset GIT_EDITOR; EDITOR=emacs git var GIT_EDITOR
> emacs
Sorry i was wrong, i missed that git var looks at $GIT_EDITOR.
So the sequence for choosing the sequencer editor is:
$GIT_SEQUENCE_EDITOR
config sequence.editor
var GIT_EDITOR
Which looks OK to me.
> > * Should git_rebase_editor be in git-rebase--interactive.sh instead
>
> Probably yes.
OK, will do.
>
> > * How should the config be called?
>
> Given that in the longer term we would be using a unified sequencer
> machinery for not just rebase-i but for am and cherry-pick, I would advise
> against calling this anything "rebase". How does "sequence.edit" sound?
>
I do not really care very much, but how about sequence.editor?
Sounds more similar to core.editor
> You need to be prepared to adjust your code to deal with new kinds of
> sequencing insns in the insn sheet and possibly a format change of the
> insn sheet itself.
I assume instruction sheet is the commented out part that looks like:
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
Currently all lines starting with # are ignored.
(They are also not written to the output when finished
which is a point I might have to change...)
Also the instructions are currently not taken from this instruction sheet.
They are all hardcoded.
Thanks for the feedback
Greetings Peter
^ permalink raw reply
* Re: [PATCH] Fix is_gitfile() for files larger than PATH_MAX
From: Phil Hord @ 2011-10-11 20:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vy5wre0n7.fsf@alter.siamese.dyndns.org>
On Tue, Oct 11, 2011 at 4:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> @@ -868,8 +868,8 @@ static int is_gitfile(const char *url)
>> return 0;
>> if (!S_ISREG(st.st_mode))
>> return 0;
>> - if (st.st_size < 10 || st.st_size > PATH_MAX)
>> - return 1;
>> + if (st.st_size < 10 || st.st_size > 9 + PATH_MAX)
>> + return 0;
>
> We are asked if the file is likely to be a single-liner "gitfile: <path>",
> and were answering yes when it is a very short file (less than 10 bytes)
> that cannot possibly even contain "gitfile: " prefix.
>
> I suspect that we can and should get rid of the "cannot be very long"
> check altogether---we do open and check the file, and after all it is not
> like we are throwing different strings as "url" argument to this function
> at random and this function needs heuristics to reject bogus input
> early. The argument is an input from the user.
>
> Quite an embarrasing bug. Thanks for spotting.
Yes, and it's _my_ embarrassing bug. I've caught this and have it in
a re-roll, but I got mired up and haven't submitted it again. I'll
try to do so today.
Phil
^ permalink raw reply
* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Jeff King @ 2011-10-11 20:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <7v39ezffq5.fsf_-_@alter.siamese.dyndns.org>
On Tue, Oct 11, 2011 at 01:14:26PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> I think we've discussed tightening it a few years ago already.
> >>
> >> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
> >> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
> >
> > Perhaps like this? Only compile tested...
>
> Not quite. There are at least three bugs in the patch.
>
> - Some subsystems use random refnames like NOTES_MERGE_PARTIAL that would
> not match "^([A-Z][A-Z]*_)?HEAD$". The rule needs to be relaxed;
Yeah, I found one of these also. I thought at first we could rename
things like NOTES_MERGE_PARTIAL, as it's more about internal
communication within a specific version of git than it is about letting
an external program peek at our state. But there really are several of
them. And I think it makes sense to keep this safety valve conservative,
and try to document existing practice rather than dictating it. I'm
worried that some porcelain or other tool is using their own all-caps
name, and that tightening it too much would be breaking that.
Your relaxed rule does match things like COMMIT_EDITMSG and NOTES_MSG,
which are obviously bogus. At the same time, I'm not sure it's a big
deal. The point of this is to restrict the lookup to a class of names
which are likely magical to git, and users should probably avoid the
magical namespace (i.e., it's still not a good idea to call your branch
"HEAD"). Something like "config" is an easy branch name to unknowingly
use. Something like "COMMIT_EDITMSG" is not.
Your rule does disallow RENAMED-REF, which is used in branch renaming.
However, I'm having trouble figuring out what it's really for. It's not
mentioned in the documentation at all, and c976d41, its origin, says
only:
This also indroduces $GIT_DIR/RENAME_REF which is a "metabranch"
used when renaming branches. It will always hold the original
sha1 for the latest renamed branch.
Additionally, if $GIT_DIR/logs/RENAME_REF exists,
all branch rename events are logged there.
But in the code, it is spelled RENAMED-REF (with a dash). And as far as
I can tell, does not actually create a reflog. And it's not documented
anywhere, so I suspect nobody is using it. Maybe it is worth switching
that name.
> - dwim_ref() can be fed "refs/heads/master" and is expected to dwim it to
> the master branch.
It looks like your code will allow any subdirectory. I had thought to
limit it to "refs/". Otherwise, my "config" example could be
"objects/pack", or "lost-found/commits", "remotes/foo", or something.
Obviously the longer the name, the smaller the possibility of an
accidental collision. But I couldn't think of any other subdirectory
into which refs should go.
-Peff
^ permalink raw reply
* Re: [PATCH] Fix is_gitfile() for files larger than PATH_MAX
From: Junio C Hamano @ 2011-10-11 20:25 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.1110111424010.32316@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The logic to check whether a file is a gitfile used the heuristics that
> the file cannot be larger than PATH_MAX. But in that case it returned the
> wrong value. Our test cases do not cover this, as the bundle files
> produced are smaller than PATH_MAX. Except on Windows.
>
> While at it, fix the faulty logic that the path stored in a gitfile cannot
> be larger than PATH_MAX-sizeof("gitfile: ").
>
> Problem identified by running the test suite in msysGit, offending commit
> identified by Jörg Rosenkranz.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> This patch should apply cleanly to 'next', which we track in
> msysgit/git.
>
> The task of adding a test case is something I leave to someone who
> wants to get involved with Git development and needs an easy way
> in.
>
> transport.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index f3195c0..57138d9 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -868,8 +868,8 @@ static int is_gitfile(const char *url)
> return 0;
> if (!S_ISREG(st.st_mode))
> return 0;
> - if (st.st_size < 10 || st.st_size > PATH_MAX)
> - return 1;
> + if (st.st_size < 10 || st.st_size > 9 + PATH_MAX)
> + return 0;
We are asked if the file is likely to be a single-liner "gitfile: <path>",
and were answering yes when it is a very short file (less than 10 bytes)
that cannot possibly even contain "gitfile: " prefix.
I suspect that we can and should get rid of the "cannot be very long"
check altogether---we do open and check the file, and after all it is not
like we are throwing different strings as "url" argument to this function
at random and this function needs heuristics to reject bogus input
early. The argument is an input from the user.
Quite an embarrasing bug. Thanks for spotting.
^ permalink raw reply
* Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-11 20:14 UTC (permalink / raw)
To: Jeff King, Michael Haggerty
Cc: git, cmn, A Large Angry SCM, Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <7vmxd7flkw.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> I think we've discussed tightening it a few years ago already.
>>
>> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
>> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
>
> Perhaps like this? Only compile tested...
Not quite. There are at least three bugs in the patch.
- Some subsystems use random refnames like NOTES_MERGE_PARTIAL that would
not match "^([A-Z][A-Z]*_)?HEAD$". The rule needs to be relaxed;
- dwim_ref() can be fed "refs/heads/master" and is expected to dwim it to
the master branch.
- These codepaths get pointer+length so that it can be told to parse only
the first 4 bytes in "HEAD:$path".
-- >8 --
Subject: [PATCH] Restrict ref-like names immediately below $GIT_DIR
We have always dwimmed the user input $string into a ref by first looking
directly inside $GIT_DIR, and then in $GIT_DIR/refs, $GIT_DIR/refs/tags,
etc., and that is what made
git log HEAD..MERGE_HEAD
work correctly. This however means that
git rev-parse config
git log index
would look at $GIT_DIR/config and $GIT_DIR/index and see if they are valid
refs.
To reduce confusion, let's not dwim a path immediately below $GIT_DIR that
is not all-caps.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
sha1_name.c | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 143fd97..5eb19c2 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -261,6 +261,25 @@ static char *substitute_branch_name(const char **string, int *len)
return NULL;
}
+static int ok_at_root_level(const char *str, int len)
+{
+ int seen_non_root_char = 0;
+
+ while (len--) {
+ char ch = *str++;
+
+ if (ch == '/')
+ return 1;
+ /*
+ * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
+ * the root level as a ref.
+ */
+ if (ch != '_' && (ch < 'A' || 'Z' < ch))
+ seen_non_root_char = 1;
+ }
+ return !seen_non_root_char;
+}
+
int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
{
char *last_branch = substitute_branch_name(&str, &len);
@@ -274,6 +293,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
unsigned char *this_result;
int flag;
+ if (p == ref_rev_parse_rules && !ok_at_root_level(str, len))
+ continue;
this_result = refs_found ? sha1_from_ref : sha1;
mksnpath(fullref, sizeof(fullref), *p, len, str);
r = resolve_ref(fullref, this_result, 1, &flag);
@@ -302,6 +323,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
char path[PATH_MAX];
const char *ref, *it;
+ if (p == ref_rev_parse_rules && !ok_at_root_level(str, len))
+ continue;
mksnpath(path, sizeof(path), *p, len, str);
ref = resolve_ref(path, hash, 1, NULL);
if (!ref)
^ permalink raw reply related
* Re: Symmetric of rebase / more intelligent cherry-pick
From: Junio C Hamano @ 2011-10-11 19:36 UTC (permalink / raw)
To: Lionel Elie Mamane; +Cc: git
In-Reply-To: <20111011155444.GB14417@capsaicin.mamane.lu>
Lionel Elie Mamane <lionel@mamane.lu> writes:
> git cherry-pick ..UPSTREAM
> *nearly* does what I want, except that it lacks rebase's intelligence
> of skipping commits that do the same textual changes as a commit
> already in the current branch.
I think in the longer term "--ignore-if-in-upstream" that is known only to
format-patch, which is the true source the intelligence of rebase you
observed comes from, should be factored out into a helper function that
can be used to filter output from get_revision() in other commands, or
perhaps get_revision() itself might want to learn it.
I say "or perhaps might" above, because I do not think the general
revision traversal machinery used by the log family (which cherry-pick's
multi-pick option relies on) has enough information to decide what the
caller means by "upstream" at the point setup_revisions() is called.
^ permalink raw reply
* [PATCH] Fix is_gitfile() for files larger than PATH_MAX
From: Johannes Schindelin @ 2011-10-11 19:25 UTC (permalink / raw)
To: git, gitster
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1276 bytes --]
The logic to check whether a file is a gitfile used the heuristics that
the file cannot be larger than PATH_MAX. But in that case it returned the
wrong value. Our test cases do not cover this, as the bundle files
produced are smaller than PATH_MAX. Except on Windows.
While at it, fix the faulty logic that the path stored in a gitfile cannot
be larger than PATH_MAX-sizeof("gitfile: ").
Problem identified by running the test suite in msysGit, offending commit
identified by Jörg Rosenkranz.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
This patch should apply cleanly to 'next', which we track in
msysgit/git.
The task of adding a test case is something I leave to someone who
wants to get involved with Git development and needs an easy way
in.
transport.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/transport.c b/transport.c
index f3195c0..57138d9 100644
--- a/transport.c
+++ b/transport.c
@@ -868,8 +868,8 @@ static int is_gitfile(const char *url)
return 0;
if (!S_ISREG(st.st_mode))
return 0;
- if (st.st_size < 10 || st.st_size > PATH_MAX)
- return 1;
+ if (st.st_size < 10 || st.st_size > 9 + PATH_MAX)
+ return 0;
fd = open(url, O_RDONLY);
if (fd < 0)
--
1.7.6.msysgit.0.584.g2cbf
^ permalink raw reply related
* Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Junio C Hamano @ 2011-10-11 18:37 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: git
In-Reply-To: <201110111956.08829.kumbayo84@arcor.de>
Peter Oberndorfer <kumbayo84@arcor.de> writes:
> Using $GIT_EDITOR or core.editor config var for this is not possible
> since it is also used to start the commit message editor for reword action.
Your tool _could_ be smart about this issue and inspect the contents to
launch a real editor when it is fed a material not for sequencing, but
that feels hacky.
> * GIT_EDITOR env var is not honored anymore after this change.
Care to explain? "git var" knows magic about a few variables like
GIT_EDITOR and GIT_PAGER.
$ git config core.editor vim
$ GIT_EDITOR=vi EDITOR=emacs git var GIT_EDITOR
vi
$ unset GIT_EDITOR; EDITOR=emacs git var GIT_EDITOR
emacs
> * Should git_rebase_editor be in git-rebase--interactive.sh instead
Probably yes.
> * How should the config be called?
Given that in the longer term we would be using a unified sequencer
machinery for not just rebase-i but for am and cherry-pick, I would advise
against calling this anything "rebase". How does "sequence.edit" sound?
You need to be prepared to adjust your code to deal with new kinds of
sequencing insns in the insn sheet and possibly a format change of the
insn sheet itself.
^ permalink raw reply
* [RESEND PATCH v3] Configurable hyperlinking in gitk
From: Jeff Epler @ 2011-10-11 18:37 UTC (permalink / raw)
To: git, Marc Branchaud, Chris Packham, Jakub Narebski,
Junio C Hamano, Paul
In-Reply-To: <20110922013101.GB26880@unpythonic.net>
Many projects use project-specific notations in changelogs to refer
to bug trackers and the like. One example is the "Closes: #12345"
notation used in Debian.
Make gitk configurable so that arbitrary strings can be turned into
clickable links that are opened in a web browser.
Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
This v3 patch didn't generate any discussion last time around (~3 weeks
ago), so I've taken the liberty of reposting it.
I'm aware of no problems with this patch, and a number of people have
commented that it is useful to them. For URLs that contain "&" and
other shell metacharacters, it *does* depend on r480f062c
"git-web--browse: avoid the use of eval" which is in next but not maint.
Since the V2 patch, I
* Renamed configuration variables to get rid of the "gitk" prefix
to encourage other git-related programs to adopt the same
functionality.
* Renamed configuration variables from cryptic "re", "sub" to less
cryptic "regexp" and "subst"
* Changed the example RE to be an ERE (no \d or \M)
* Documented that these are POSIX EREs; hopefully that's OK. I see
in CodingGuidelines that in git itself "a subset of BREs" are used,
so maybe even this is too much power. And hopefully tcl's
re_syntax really is close enough to an ERE superset that this isn't
a terrible lie about the initial implementation either.
* Added a Signed-Off-By, since I've had a number of positive feedbacks
and the only problems I've heard of (since patch v2) are the ones
related to 'eval' in git-web--browse.
In v2 of the patch, I had fixed a problem with %-signs in URLs and
changed the documentation example.
Documentation/config.txt | 30 +++++++++++++++++-
gitk-git/gitk | 75 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae9913b..ffc9ccf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1064,6 +1064,10 @@ All gitcvs variables except for 'gitcvs.usecrlfattr' and
is one of "ext" and "pserver") to make them apply only for the given
access method.
+gitk.browser::
+ Specify the browser that will be used to open links generated by
+ 'linkify' configuration options.
+
grep.lineNumber::
If set to true, enable '-n' option by default.
@@ -1317,6 +1321,28 @@ interactive.singlekey::
setting is silently ignored if portable keystroke input
is not available.
+linkify.<name>.regexp::
+ Specify a regular expression in the POSIX Extended Regular Expression
+ syntax defining a class of strings to automatically convert to
+ hyperlinks. This regular expression many not span multiple lines.
+ You must also specify 'linkify.<name>.subst'.
+
+linkify.<name>.subst::
+ Specify a substitution that results in the target URL for the
+ related regular expression. Back-references like '\1' refer
+ to capturing groups in the associated regular expression.
+ You must also specify 'linkify.<name>.regexp'.
++
+For example, to automatically link from Debian-style "Closes: #nnnn"
+message to the Debian BTS,
++
+--------
+ git config linkify.debian-bts.regexp '#([1-9][0-9]*)'
+ git config linkify.debian-bts.subst 'http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=\1'
+--------
++
+Currently, only linkgit:gitk[1] converts strings to links in this fashion.
+
log.abbrevCommit::
If true, makes linkgit:git-log[1], linkgit:git-show[1], and
linkgit:git-whatchanged[1] assume `\--abbrev-commit`. You may
@@ -1870,5 +1896,5 @@ user.signingkey::
web.browser::
Specify a web browser that may be used by some commands.
- Currently only linkgit:git-instaweb[1] and linkgit:git-help[1]
- may use it.
+ Currently only linkgit:git-instaweb[1], linkgit:gitk[1],
+ and linkgit:git-help[1] may use it.
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..9db5525 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -6684,7 +6684,7 @@ proc commit_descriptor {p} {
# append some text to the ctext widget, and make any SHA1 ID
# that we know about be a clickable link.
proc appendwithlinks {text tags} {
- global ctext linknum curview
+ global ctext linknum curview linkmakers
set start [$ctext index "end - 1c"]
$ctext insert end $text $tags
@@ -6699,6 +6699,30 @@ proc appendwithlinks {text tags} {
setlink $linkid link$linknum
incr linknum
}
+
+ if {$linkmakers == {}} return
+
+ set link_re {}
+ foreach {re rep} $linkmakers { lappend link_re $re }
+ set link_re "([join $link_re {)|(}])"
+
+ set ee 0
+ while {[regexp -indices -start $ee -- $link_re $text l]} {
+ set s [lindex $l 0]
+ set e [lindex $l 1]
+ set linktext [string range $text $s $e]
+ incr e
+ set ee $e
+
+ foreach {re rep} $linkmakers {
+ if {![regsub $re $linktext $rep linkurl]} continue
+ $ctext tag delete link$linknum
+ $ctext tag add link$linknum "$start + $s c" "$start + $e c"
+ seturllink $linkurl link$linknum
+ incr linknum
+ break
+ }
+ }
}
proc setlink {id lk} {
@@ -6726,6 +6750,53 @@ proc setlink {id lk} {
}
}
+proc get_link_config {} {
+ if {[catch {exec git config -z --get-regexp {^linkify\.}} linkers]} {
+ return {}
+ }
+
+ set linktypes [list]
+ foreach item [split $linkers "\0"] {
+ if {$item == ""} continue
+ if {![regexp {linkify\.(\S+)\.(regexp|subst)\s(.*)} $item _ k t v]} {
+ continue
+ }
+ set linkconfig($t,$k) $v
+ if {$t == "regexp"} { lappend linktypes $k }
+ }
+
+ set linkmakers [list]
+ foreach k $linktypes {
+ if {![info exists linkconfig(subst,$k)]} {
+ puts stderr "Warning: link `$k' is missing a substitution string"
+ } elseif {[catch {regexp -inline -- $linkconfig(regexp,$k) ""} err]} {
+ puts stderr "Warning: link `$k': $err"
+ } else {
+ lappend linkmakers $linkconfig(regexp,$k) $linkconfig(subst,$k)
+ }
+ unset linkconfig(regexp,$k)
+ unset -nocomplain linkconfig(subst,$k)
+ }
+ foreach k [array names linkconfig] {
+ regexp "subst,(.*)" $k _ k
+ puts stderr "Warning: link `$k' is missing a regular expression"
+ }
+ set linkmakers
+}
+
+proc openlink {url} {
+ exec git web--browse --config=gitk.browser $url &
+}
+
+proc seturllink {url lk} {
+ set qurl [string map {% %%} $url]
+ global ctext
+ $ctext tag conf $lk -foreground blue -underline 1
+ $ctext tag bind $lk <1> [list openlink $qurl]
+ $ctext tag bind $lk <Enter> {linkcursor %W 1}
+ $ctext tag bind $lk <Leave> {linkcursor %W -1}
+}
+
proc appendshortlink {id {pre {}} {post {}}} {
global ctext linknum
@@ -11693,6 +11764,8 @@ if {[tk windowingsystem] eq "win32"} {
focus -force .
}
+set linkmakers [get_link_config]
+
getcommits {}
# Local variables:
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
From: Jakub Narebski @ 2011-10-11 18:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Drew Northup, Jonathan Nieder
In-Reply-To: <7vvcrvfmg4.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> On Tue, 11 Oct 2011, Junio C Hamano wrote:
>>
>>> I probably do not have time to look into this, but just FYI my trial merge
>>> to 'pu' of this topic is failing like this:
>>>
>>> asciidoc: ERROR: gitweb.conf.txt: line 484: illegal style name: Default: ()
>>> asciidoc: ERROR: gitweb.conf.txt: line 494: illegal style name: Default: 300
>>
>> Damn, I thought I have fixed that. This probably depends on AsciiDoc
>> version ("make doc" on 'master' generates a few _warnings_ for me related
>> to similar situation), but the problem is with
>>
>> [Default: <value>]
>>
>> that was copied from gitweb/README. But [<sth>] is an attribute list
>> (style name in simplest form), used more often in newer AsciiDoc.
>>
>> So either we have to escape '[' and ']', i.e. use {startsb} and {endsb},
>> which would reduce human-friendliness, or move to different way of marking
>> default values, e.g. _italic_.
>>
>> What do you think?
>
> What do the other documents in the directory this file lives say? I think
> we explain what the variables does, and add "defaults to false" or
> somesuch in the text, without any funny mark-up.
O.K., will do.
Now that reminds me that in a few situations gitweb.conf.txt uses literary
description "defaults to sth"... I'll make the rest consistent with this.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Phil Hord @ 2011-10-11 18:15 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: git
In-Reply-To: <201110111956.08829.kumbayo84@arcor.de>
On Tue, Oct 11, 2011 at 1:56 PM, Peter Oberndorfer <kumbayo84@arcor.de> wrote:
> i wrote a (not yet released) git rebase -i helper that allows to order commits
> by drag/drop and allows to select the action from a combo box.
> (written in Qt)
> See http://i55.tinypic.com/2d94gg0.jpg for how it currently looks like. :-)
> No more typos, no more lost commit by cutting without pasting...
[+1]
>
> To integrate this properly into git i need something like this patch.
>
> Open questions/problems:
> * GIT_EDITOR env var is not honored anymore after this change.
> Help from somebody with more bash knowledge is highly appreciated!
>
> * Should git_rebase_editor be in git-rebase--interactive.sh instead
> (since it is only used there)
>
> * How should the config be called?
> It is not directly used during rebase, only during rebase -i
> that might not be fully clear from the config name.
>
> * Better config.txt description?
>
> Thanks,
> Greetings Peter
>
> PS: My tool will hopefully be released soon.
> Cleanup code, test(lin/ win), write some doc (how to use with git),
> choose name :-), choose license...
>
> Documentation/config.txt | 6 ++++++
> git-rebase--interactive.sh | 2 +-
> git-sh-setup.sh | 13 +++++++++++++
> 3 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 03296b7..1d9ae79 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1591,6 +1591,12 @@ rebase.stat::
> Whether to show a diffstat of what changed upstream since the last
> rebase. False by default.
>
> +rebase.editor::
> + Text editor used by git rebase -i for editing the rebasse todo file.
s/rebasse/rebase/
> cp "$todo" "$todo".backup
> -git_editor "$todo" ||
> +git_rebase_editor "$todo" ||
> die_abort "Could not execute editor"
Maybe something like this would work:
git_rebase_editor "$todo" ||
git_editor "$todo" ||
die_abort "Could not execute editor"
If git_rebase_editor call returns an error (non-zero exit code), then
git_editor will be invoked. If that also returns an error, then the
die_abort is called.
I think this will allow your env:GIT_EDITOR to work as expected.
Phil
^ permalink raw reply
* Re: [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-11 18:07 UTC (permalink / raw)
To: Jeff King, Michael Haggerty
Cc: git, cmn, A Large Angry SCM, Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <7vr52jfm8i.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> I wonder if the right solution is for us to be more picky about what can
>> be found in $GIT_DIR. Maybe matching all-uppercase, or starting with
>> "refs/", which I think would match existing convention?
>
> I think we've discussed tightening it a few years ago already.
>
> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
Perhaps like this? Only compile tested...
sha1_name.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index ba976b4..5effb1a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -261,6 +261,23 @@ static char *substitute_branch_name(const char **string, int *len)
return NULL;
}
+static int is_kind_of_head(const char *str)
+{
+ int len = strlen(str) - 4;
+ if (len < 0 || strcmp(str + len, "HEAD"))
+ return 0;
+ if (!len)
+ return 1;
+ if (str[--len] != '_' || !len)
+ return 0;
+ while (len) {
+ char ch = str[--len];
+ if (ch < 'A' || 'Z' < ch)
+ return 0;
+ }
+ return 1;
+}
+
int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
{
char *last_branch = substitute_branch_name(&str, &len);
@@ -274,6 +291,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
unsigned char *this_result;
int flag;
+ if (p == ref_rev_parse_rules && !is_kind_of_head(str))
+ continue;
this_result = refs_found ? sha1_from_ref : sha1;
mksnpath(fullref, sizeof(fullref), *p, len, str);
r = resolve_ref(fullref, this_result, 1, &flag);
@@ -302,6 +321,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
char path[PATH_MAX];
const char *ref, *it;
+ if (p == ref_rev_parse_rules && !is_kind_of_head(str))
+ continue;
mksnpath(path, sizeof(path), *p, len, str);
ref = resolve_ref(path, hash, 1, NULL);
if (!ref)
^ permalink raw reply related
* [RFC/WIP PATCH] Use config value rebase.editor as editor when starting git rebase -i
From: Peter Oberndorfer @ 2011-10-11 17:56 UTC (permalink / raw)
To: git
If rebase.editor is not set interactive rebase falls back
to the default editor.
With this change is it possible to have a separate
(possibly graphical) editor that helps the user
during a interactive rebase.
Using $GIT_EDITOR or core.editor config var for this is not possible
since it is also used to start the commit message editor for reword action.
Signed-off-by: Peter Oberndorfer <kumbayo84@arcor.de>
---
Hi,
i wrote a (not yet released) git rebase -i helper that allows to order commits
by drag/drop and allows to select the action from a combo box.
(written in Qt)
See http://i55.tinypic.com/2d94gg0.jpg for how it currently looks like. :-)
No more typos, no more lost commit by cutting without pasting...
To integrate this properly into git i need something like this patch.
Open questions/problems:
* GIT_EDITOR env var is not honored anymore after this change.
Help from somebody with more bash knowledge is highly appreciated!
* Should git_rebase_editor be in git-rebase--interactive.sh instead
(since it is only used there)
* How should the config be called?
It is not directly used during rebase, only during rebase -i
that might not be fully clear from the config name.
* Better config.txt description?
Thanks,
Greetings Peter
PS: My tool will hopefully be released soon.
Cleanup code, test(lin/ win), write some doc (how to use with git),
choose name :-), choose license...
Documentation/config.txt | 6 ++++++
git-rebase--interactive.sh | 2 +-
git-sh-setup.sh | 13 +++++++++++++
3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 03296b7..1d9ae79 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1591,6 +1591,12 @@ rebase.stat::
Whether to show a diffstat of what changed upstream since the last
rebase. False by default.
+rebase.editor::
+ Text editor used by git rebase -i for editing the rebasse todo file.
+ The value is meant to be interpreted by the shell when it is used.
+ When not configured the default commit message editor is used instead.
+ See "core.editor"
+
rebase.autosquash::
If set to true enable '--autosquash' option by default.
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94f36c2..0f3b569 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -832,7 +832,7 @@ has_action "$todo" ||
die_abort "Nothing to do"
cp "$todo" "$todo".backup
-git_editor "$todo" ||
+git_rebase_editor "$todo" ||
die_abort "Could not execute editor"
has_action "$todo" ||
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 8e427da..303fb96 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -113,6 +113,19 @@ git_editor() {
eval "$GIT_EDITOR" '"$@"'
}
+git_rebase_editor() {
+ if test -z "${GIT_REBASEI_EDITOR:+set}"
+ then
+ GIT_REBASEI_EDITOR="$(git config rebase.editor)"
+ if [ -z "$GIT_REBASEI_EDITOR" ]
+ then
+ GIT_REBASEI_EDITOR="$(git var GIT_EDITOR)" || return $?
+ fi
+ fi
+
+ eval "$GIT_REBASEI_EDITOR" '"$@"'
+}
+
git_pager() {
if test -t 1
then
--
1.7.7.215.gfef80
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox