Re: [PATCH 1/2] help: introduce option --command-only

2016-08-19 Thread Remi Galan Alfonso
Hi Ralf, Ralf Thielow writes: > [...] > +test_expect_success "works for commands and guides by default" " > +git help status && > +git help revisions > +" > + > +test_expect_success "--command-only does not work for guides" " > +git help --command-only status && > +

Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

2016-08-17 Thread Remi Galan Alfonso
Stephen Morton writes: > [snip] > On 2016-08-16 4:44 AM, Remi Galan Alfonso wrote: >> Hi Stephen, >> >> Stephen Morton writes: >>> +if (multiple_commits) >>> + advise(_("after resolving the co

Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path

2016-08-17 Thread Remi Galan Alfonso
Junio C Hamano writes: > Remi Galan Alfonso > writes: > >> Johannes Schindelin writes: >>> Hi Rémi, >>> >>> On Tue, 16 Aug 2016, Remi Galan Alfonso wrote: >>> >>> > Johannes Schindelin writes: >>> > > diff --g

Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path

2016-08-16 Thread Remi Galan Alfonso
Johannes Schindelin writes: > Hi Rémi, > > On Tue, 16 Aug 2016, Remi Galan Alfonso wrote: > > > Johannes Schindelin writes: > > > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh > > > index 5e3fb3a..f1f9aee 100755 > > > ---

Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path

2016-08-16 Thread Remi Galan Alfonso
Hi Johannes, Johannes Schindelin writes: > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh > index 5e3fb3a..f1f9aee 100755 > --- a/t/t1350-config-hooks-path.sh > +++ b/t/t1350-config-hooks-path.sh > @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of > sp

Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

2016-08-16 Thread Remi Galan Alfonso
Hi Stephen, Stephen Morton writes: > +if (multiple_commits) > + advise(_("after resolving the conflicts, > mark the corrected paths with 'git add ' or 'git rm '\n" > +"then continue with 'git %s > --con

Re: [PATCHv2] checkout: do not mention detach advice for explicit --detach option

2016-08-12 Thread Remi Galan Alfonso
Stefan Beller writes: > +test_expect_success 'no advice given for explicit detached head state' ' > +git config advice.detachedHead false && > +git checkout child && > +git checkout --detach HEAD >expect && > +git config advice.detachedHead true && Is there a reaso

Re: Bug? git rebase -i --autostash fails to restore working files

2016-07-28 Thread Remi Galan Alfonso
Hi Phillip, Phillip Wood writes: > When running ‘git rebase -i --autostash’ if the editor fails then the > rebase fails but stash is not applied so the working files are not > restored. Running ‘git rebase --abort’ replies that there’s no rebase > in progress. If you notice what’s happened it’s n

Re: [PATCH v1 2/3] convert: modernize tests

2016-07-26 Thread Remi Galan Alfonso
Hi Lars, Sorry, minor nit that I noticed a couple of days ago but didn't comment on the moment and forgot until now. Lars Schneider wrote: > Use `test_config` to set the config, check that files are empty with > `test_must_be_empty`, compare files with `test_cmp`, and remove spaces > after ">".

Re: [PATCH v3 4/4] archive-tar: drop return value

2016-06-24 Thread Remi Galan Alfonso
Hi Peff, Jeff King writes: > @@ -413,7 +411,7 @@ static int write_tar_archive(const struct archiver *ar, > { > int err = 0; > > -err = write_global_extended_header(args); > +write_global_extended_header(args); > if (!err) > err = write_archive

Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison

2016-06-08 Thread Remi Galan Alfonso
Hi Samuel, Samuel GROOT writes: > +test_cmp_noorder () { > +sort "$1" >"$1_noorder" > +sort "$2" >"$2_noorder" > +test_cmp $1 $2 You meant `test_cmp "$1_noorder" "$2_noorder"`, I guess. Thanks, Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the

Re: [PATCH] completion: complete --move for git branch

2016-06-07 Thread Remi Galan Alfonso
Ville Skyttä writes: > On Mon, Jun 6, 2016 at 5:16 PM, Remi Galan Alfonso > wrote: > > > > Hi, > > > > Ville Skyttä writes: > > > while [ $c -lt $cword ]; do > > > i="${words[c]}" > > >

Re: [PATCH] completion: complete --move for git branch

2016-06-06 Thread Remi Galan Alfonso
Hi, Ville Skyttä writes: > while [ $c -lt $cword ]; do > i="${words[c]}" > case "$i" in > --d|-m)only_local_ref="y" ;; > --r)has_r="y" ;; > +-d|-m|--move)only_local_ref="y" ;; "Whil

Re: [RFC/PATCH] push: deny policy to prevent pushes to unwanted remotes.

2016-06-06 Thread Remi Galan Alfonso
Hi Antoine, Antoine Queru writes: > [...] > +For example, if we set up the configuration variables like this: > + > +--- > +git config --add remote.pushBlacklist repository.com > +git config --add remote.pushWhitelist repository.com/Special_Folder > +--

Re: [PATCH 0/2] strbuf: improve API

2016-05-30 Thread Remi Galan Alfonso
William Duclot writes: > This patch series implements an improvment of the strbuf API, allowing > strbuf to use preallocated memory. This makes strbuf fit to be used > in performance-critical operations. > > The first patch is simply a preparatory work, adding tests for > existing strbuf implemen

Re: [RFC/PATCH 1/2] log: add "log.showsignature" configuration variable

2016-05-26 Thread Remi Galan Alfonso
Mehul Jain writes: > Hi Remi, > > Thanks for your input. > > On Thu, May 26, 2016 at 7:12 PM, Remi Galan Alfonso > wrote: > > Hi Mehul, > > > > Mehul Jain writes: > >> When log.showsignature set true, "git log" and "git show

Re: [RFC/PATCH 1/2] log: add "log.showsignature" configuration variable

2016-05-26 Thread Remi Galan Alfonso
Hi Mehul, Mehul Jain writes: > People may want to always use "--show-signature" while using "git log" > or "git show". > > When log.showsignature set true, "git log" and "git show" will behave 'When log.showsignature is set to true' ? > as "--show-signature" was given to them. s/as/as if > S

Re: [PATCH 2/2] submodule update: learn `--[no-]recommend-shallow` option

2016-05-26 Thread Remi Galan Alfonso
You forgot to update from recommend-depth to recommend-shallow Stefan Beller writes: > [...] > 'git submodule' [--quiet] init [--] [...] > 'git submodule' [--quiet] deinit [-f|--force] (--all|[--] ...) > 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] > - [-f|

Re: [PATCH 1/2] submodule-config: keep shallow recommendation around

2016-05-26 Thread Remi Galan Alfonso
Hi Stefan, Stefan Beller writes: > [...] > @ -353,6 +354,15 @@ static int parse_config(const char *var, const char > *value, void *data) > else if (parse_submodule_update_strategy(value, > &submodule->update_strategy) < 0) >

Re: possible bug of git stash deleting uncommitted files in corner case

2016-04-22 Thread Remi Galan Alfonso
Junio C Hamano writes: > Remi Galan Alfonso > writes: > > > Daniele Segato wrote: > > ... > >> git version 1.9.1 > > > > Contrary to what I expected, this seems to still be the case with: > > $ git --version > > git version 2.8.0.rc2 &

Re: possible bug of git stash deleting uncommitted files in corner case

2016-04-22 Thread Remi Galan Alfonso
Daniele Segato wrote: > git init > echo 'X' > foo > git add foo > git commit -m 'foo file committed' > > rm foo > mkdir foo > echo 'B' > foo/bar > > # git status > > git stash > > at this point stash deleted the "bar" file, in his case all the work > on the previous couple of hours, but he did

Re: Ambiguous sha-1 during a rebase

2016-04-14 Thread Remi Galan Alfonso
Matthieu Moy writes: > Mike Hommey writes: > > > Yeah, that definitely is a weird corner case. Interestingly, it was > > complaining about "error: short SHA1 e34ff55 is ambiguous." when apply > > *other* commits that were in the list prior to it, > > I think it did before: when normalizing the

Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}

2016-04-06 Thread Remi Galan Alfonso
elena petrashen wrote: > On Tue, Apr 5, 2016 at 12:53 AM, Remi Galan Alfonso > wrote: > > elena petrashen wrote: > >> On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso > >> wrote: > >> > Elena Petrashen wrote: > >> >> +vo

Re: [RFC/PATCH] git.spec: fix changelog dates

2016-04-05 Thread Remi Galan Alfonso
Michael J Gruber wrote: > Remi Galan Alfonso venit, vidit, dixit 05.04.2016 14:28: > > Michael J Gruber wrote: > >> A few changelog entries have inconsistent dates, which rpmlint reports > >> as errors. > >> > >> Fix them based on these assumptions:

Re: [RFC/PATCH] git.spec: fix changelog dates

2016-04-05 Thread Remi Galan Alfonso
Michael J Gruber wrote: > A few changelog entries have inconsistent dates, which rpmlint reports > as errors. > > Fix them based on these assumptions: > - It's easier to mistype a number than a weekday abbreviation. > - changelog date must be before git commit date > - The mistyped date is just a

Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}

2016-04-04 Thread Remi Galan Alfonso
elena petrashen wrote: > On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso > wrote: > > Elena Petrashen wrote: > >> +void delete_branch_advice(const char *name, const char *ref) > >> +{ > >> +const char fmt[] = > >> +"\nNot

Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}

2016-03-31 Thread Remi Galan Alfonso
Elena Petrashen wrote: > +void delete_branch_advice(const char *name, const char *ref) > +{ > +const char fmt[] = > +"\nNote: to restore the deleted branch:\n\ngit branch %s %s\n"; Shouldn't that be marked for translation, like is done with the other strings? Thanks, Rémi -- To u

Re: [RFC/GSOC] Git Beginner | Warnings for potentially destructive commands (v2)

2016-03-31 Thread Remi Galan Alfonso
Sidhant Sharma wrote: > $ ggit push --force > > Pushing changes to $REMOTE/$BRANCH > [WARNING] You are about to purge commits from the $REMOTE/$BRANCH branch and > overwrite it's history to match yours. > For instance, assume the following history exists where 'origin' is > a configured remote an

Re: Regarding GSoC 2016

2016-03-25 Thread Remi Galan Alfonso
Mehul Jain wrote: > On Thu, Mar 24, 2016 at 11:13 PM, Saurabh Jain > wrote: > > The proposal can be accessed here. > > > 5mWF_cqoZS6IUvGr6CVu5SmkbZk/edit?usp=sharing> > > It's broken I guess. Not sure what you mean by that, personally I ha

Re: [PATCH] rebase: accept indented comments (fixes regression)

2015-09-30 Thread Remi Galan Alfonso
Matthieu Moy writes: > With Git <2.0.6, 'git rebase' used to accept lines starting with > whitespaces followed with '#' as a comment. This was broken by > 804098b (git rebase -i: add static check for commands and SHA-1, > 2015-06-29), which introduced additional checks on the TODO-list using > "gi

Re: [PATCH v5] git-p4: add config git-p4.pathEncoding

2015-09-04 Thread Remi Galan Alfonso
Lars Schneider writes: > > Hi Lars, > > > > Lars Schneider writes: > > > >> +test_expect_success 'Clone repo containing iso8859-1 encoded paths with > >> git-p4.pathEncoding' ' > >> + > >> +test_when_finished cleanup_git && > >> +( > >> +cd "$git" && > >> +

Re: [PATCH v5] git-p4: add config git-p4.pathEncoding

2015-09-03 Thread Remi Galan Alfonso
Hi Lars, Lars Schneider writes: > +test_expect_success 'Clone repo containing iso8859-1 encoded paths with > git-p4.pathEncoding' ' > + > +test_when_finished cleanup_git && > +( > +cd "$git" && > +git init . && > +git config git-p4

Re: [PATCH v8] git-p4: Obey core.ignorecase when using P4 client specs

2015-08-28 Thread Remi Galan Alfonso
Hi, Lars Schneider writes: > Fix this by using the path case that appears first in lexicographical > order when core.ignorcase is set to true. This behavior is consistent s/core.ignorcase/core.ignorecase > with "p4" and "p4v". Thanks, Rémi -- To unsubscribe from this list: send the line "unsub

Re: [PATCH] http: add support for specifying the SSL version

2015-08-12 Thread Remi Galan Alfonso
Hello, Elia Elia Pinto writes: > +if (ssl_version != NULL && *ssl_version) { > +if (!strcmp(ssl_version,"tlsv1")) { > +sslversion = CURL_SSLVERSION_TLSv1; > +} else if (!strcmp(ssl_version,"sslv2")) { > +sslve

Re: [PATCH] fetch: add configuration for making --all default

2015-07-17 Thread Remi Galan Alfonso
Hi, Øystein Walle writes: > +fetch.all:: > +If true, fetch will automatically behave as if the `--all` > +option was given on the command line uness a remote was given. The > +default is false. s/uness/unless > +test_expect_success 'git fetch (fetch.all = true)' ' > +

Re: [PATCH 2/4] log: add --count option to git log

2015-07-02 Thread Remi Galan Alfonso
Lawrence Siebert writes: > +prepare_revision_walk(rev); There's trailing whitespace here. (See my message in 3/4) Rémi -- 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://vg

Re: [PATCH 1/4] list-object: add get_commit_count function

2015-07-02 Thread Remi Galan Alfonso
Lawrence Siebert writes: > +get_commit_count(&revs); There's trailing whitespace here. (See my message in 3/4) Rémi -- 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

Re: [PATCH 3/4] log --count: added test

2015-07-02 Thread Remi Galan Alfonso
Hi Lawrence, Lawrence Siebert writes: > +test_expect_success 'log --count' ' > +git log --count HEAD > actual && > +gitrev-list --count HEAD > expect && Why the huge space here between 'git' and 'rev-list'? Also no space after the redirection ('>'), it should be '>actual'

Re: [PATCH 0/3] rebase -i: drop, missing commits and static checks

2015-06-30 Thread Remi Galan Alfonso
Matthieu Moy writes: > Remi Galan Alfonso writes: > > > Galan Rémi writes: > >> Shouldn't all the checking also be called in a 'rebase --continue', > >> considering that it can be called after a 'rebase --edit-todo' ? > >> (Ri

Re: [PATCH 0/3] rebase -i: drop, missing commits and static checks

2015-06-30 Thread Remi Galan Alfonso
Matthieu Moy writes: > Hi, > > Here are a few fixes to squash into the commits of the series. Other > than that, the series looks good to me. > > Junio: do you prefer a reroll or do you want to apply locally? > > Matthieu Moy (3): > fixup! git rebase -i: add static check for commands and SHA-

Re: [PATCH] commit: add commit.signoff config option

2015-06-25 Thread Remi Galan Alfonso
Caio Marcelo de Oliveira Filho writes: > +test_expect_success 'commit.signoff config option' ' > +git config commit.signoff true && > +echo "yet another content *narf*" >> foo && > +echo "zort" | git commit -F - foo && > +git cat-file commit HEAD | sed "1,/^\$/d" >

Re: [PATCHv6 1/3] git-rebase -i: add command "drop" to remove a commit

2015-06-23 Thread Remi Galan Alfonso
Junio C Hamano writes: > Matthieu Moy writes: > > > Remi Galan Alfonso writes: > > > >> I think that the indentation on its own is enough to avoid confusion > >>> test_rebase_end () { > >>> test_when_finished "git c

Re: [PATCHv6 3/3] git rebase -i: add static check for commands and SHA-1

2015-06-23 Thread Remi Galan Alfonso
Junio C Hamano writes: > Galan Rémi writes: > > > I used: > >read -r command sha1 rest < >$line > >EOF > > because > >printf '%s' "$line" | read -r command sha1 rest > > doesn't work (the 3 variables have no value as a result). > > There might be a better way to do this, but

Re: [PATCHv6 1/3] git-rebase -i: add command "drop" to remove a commit

2015-06-23 Thread Remi Galan Alfonso
Junio C Hamano writes: > > Galan Rémi writes: > > > > > +test_rebase_end () { > > > +test_when_finished "git checkout master && > > > +git branch -D $1 && > > > > Is this one guaranteed to succeed? Do we want to consider it a > > failure to remove "$1" (e.g. dropTest)? > > >

Re: [PATCH v7 4/5] bisect: add the terms old/new

2015-06-23 Thread Remi Galan Alfonso
Matthieu Moy writes: > Signed-off-by: Matthieu Moy > Signed-off-by: Matthieu Moy Sounds like you went all out with this patch. Rémi -- 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

Re: [PATCHv6 1/3] git-rebase -i: add command "drop" to remove a commit

2015-06-23 Thread Remi Galan Alfonso
Matthieu Moy writes: > Remi Galan Alfonso writes: > > > Eric Sunshine writes: > >> > +test_rebase_end () { > >> > + test_when_finished "git checkout master && > >> > + git branch -D $1 && > >> > +

Re: [PATCHv6 1/3] git-rebase -i: add command "drop" to remove a commit

2015-06-23 Thread Remi Galan Alfonso
Eric Sunshine writes: > > +test_rebase_end () { > > + test_when_finished "git checkout master && > > + git branch -D $1 && > > + test_might_fail git rebase --abort" && > > + git checkout -b $1 master > > +} > > The way this is indented makes it difficult to see that lines

Re: [PATCH] format-patch: introduce format.outputDirectory configuration

2015-06-19 Thread Remi Galan Alfonso
Alexander Kuleshov writes: > +test_expect_success "format-patch format.outputDirectory option" ' > + git config format.outputDirectory "patches/" && > + git format-patch master..side && > + cnt=$(ls | wc -l) && > + echo $cnt && > + test $cnt = 3 && > + git config --unset format.outputDirectory > +

Re: [PATCH 2/3] Move unsigned long option parsing out of pack-objects.c

2015-06-19 Thread Remi Galan Alfonso
Charles Bailey writes: > test_expect_success 'long options' ' > - test-parse-options --boolean --integer 1729 --boolean --string2=321 \ > - --verbose --verbose --no-dry-run --abbrev=10 --file fi.le\ > - --obsolete > output 2> output.err && > + test-parse-options --boolean --integer 1729 --unsigned

Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity

2015-06-19 Thread Remi Galan Alfonso
Duy Nguyen writes: > + sb.buf[0] = 'Z'; > + printf("%c\n", strbuf_slopbuf[0]); > + return 0; > startup_info = &git_startup_info; I might be wrong, but I definitely think that this printf and return 0 are some debug lines that you forgot to remove. Rémi -- To unsubscribe from this list: send th

Re: What's cooking in git.git (Jun 2015, #04; Tue, 16)

2015-06-17 Thread Remi Galan Alfonso
Junio C Hamano writes: > * gr/rebase-i-drop-warn (2015-06-01) 2 commits > - git rebase -i: warn about removed commits > - git-rebase -i: add command "drop" to remove a commit > > Add "drop commit-object-name subject" command as another way to > skip replaying of a commit in "rebase -i", and then p

Re: [PATCH/RFCv5 3/3] git rebase -i: add static check for commands and SHA-1

2015-06-13 Thread Remi Galan Alfonso
Matthieu Moy writes: > Remi Galan Alfonso writes: > > > It is mainly because here the SHA-1 is a long one (40 chars) > > OK, but then the minimum would be to add a comment saying that. > > Now, this makes me wonder why you are doing the check after the sha1 > exp

Re: [PATCH/RFCv5 2/3] git rebase -i: warn about removed commits

2015-06-10 Thread Remi Galan Alfonso
Matthieu Moy writes: > Remi Galan Alfonso writes: > > > Matthieu Moy writes: > >> > +warn_file "$todo".miss > >> > >> I would find it more elegant with less intermediate files, like > >> > >> git rev-

Re: [PATCH/RFCv5 3/3] git rebase -i: add static check for commands and SHA-1

2015-06-10 Thread Remi Galan Alfonso
> > +git stripspace --strip-comments | > > +while read -r command sha1 rest > > +do > > +case $command in > > +''|noop|x|"exec") > > +;; > > +pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f) > > +

Re: [PATCH/RFCv5 2/3] git rebase -i: warn about removed commits

2015-06-10 Thread Remi Galan Alfonso
Matthieu Moy writes: > > +warn_file "$todo".miss > > I would find it more elegant with less intermediate files, like > > git rev-list $opt <"$todo".miss | while read -r line > do > warn " - $line" > done I am not really sure since I also use warn_file to display

Re: [PATCH/RFCv2 2/2] git rebase -i: warn about removed commits

2015-06-06 Thread Remi Galan Alfonso
I'm going to try to change the die_abort in this patch by a die, so that the user can use rebase --edit-todo afterward. This way, adding the checking on the SHA-1 for the 'drop' command (discussed in 1/2) (and also maybe the other commands requiring a correct SHA-1 corresponding to a commit) to the

Re: [PATCH/RFCv2 2/2] git rebase -i: warn about removed commits

2015-06-03 Thread Remi Galan Alfonso
Eric Sunshine writes: > > +test_expect_success 'rebase -i respects rebase.missingCommitsCheck=ignore' > > ' > > + test_config rebase.missingCommitsCheck ignore && > > + test_when_finished "git checkout master && > > + git branch -D tmp2" && > > Strange indentation. Co

Re: [PATCH/RFCv4 1/2] git-rebase -i: add command "drop" to remove a commit

2015-06-03 Thread Remi Galan Alfonso
Junio C Hamano writes: > But what if you got this back after the user edits? > > drop > pick 2c9c1c5 gostak: distim doshes > drop e3b601d pull: use git-rev-parse... > edit eb2a8d9 pull: handle git-fetch'... > > [...] > Did the user tried to drop something else but the > object na

Re: [PATCH/RFCv4 2/2] git rebase -i: warn about removed commits

2015-06-03 Thread Remi Galan Alfonso
Matthieu Moy writes: > You're using $1 and $2 only to redirect input and output. I would find > it more elegant to write todo_list_to_sha_list as a filter, and do the > redirection in the call site (to keep the option of using > todo_list_to_sha_list in a pipe). If I understood correctly, then th

Re: [PATCH/RFCv4 2/2] git rebase -i: warn about removed commits

2015-06-03 Thread Remi Galan Alfonso
Galan Rémi writes: > +comm -2 -3 "$todo".oldsha1 "$todo".newsha1 >"$todo".miss > + > +# Make the list user-friendly > +opt="--no-walk=sorted --format=oneline --abbrev-commit > --stdin" > +git rev-list $opt <"$todo".miss >"$todo".miss

Re: [PATCH/RFCv2 1/2] git-rebase -i: add command "drop" to remove a commit

2015-06-03 Thread Remi Galan Alfonso
Junio C Hamano writes: > As long as what is given to 'drop' > is checked when it matters (e.g. when the code in patch 2/2 tries > see if some commits in the original list are no longer there in > order to warn sees "drop foo bar" where "foo" is obviously not an > object name in the original l

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

2015-06-03 Thread Remi Galan Alfonso
> Matthieu Moy writes: > > Ideally, you would check the list of commits displayed too. If the > > commits sha1 are stable, this should be easy to do. If it's too hard to > > test, I'd say its not worth the trouble, but others may disagree. > > Originally I chose not to check if the SHA-1 were

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

2015-06-03 Thread Remi Galan Alfonso
Matthieu Moy writes: > Ideally, you would check the list of commits displayed too. If the > commits sha1 are stable, this should be easy to do. If it's too hard to > test, I'd say its not worth the trouble, but others may disagree. Originally I chose not to check if the SHA-1 were corrects since

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

2015-06-02 Thread Remi Galan Alfonso
Matthieu Moy writes: > > Galan Rémi writes: > > +# Sort the SHA-1 and compare them > > +echo "$(sort -u "$todo".oldsha1)" >"$todo".oldsha1 > > +echo "$(sort -u "$todo".newsha1)" >"$todo".newsha1 > > Useless uses of echo. > > echo $(foo) -> foo In

Re: [PATCH/RFCv3 1/2] git-rebase -i: add command "drop" to remove a commit

2015-06-02 Thread Remi Galan Alfonso
Matthieu Moy : writes > > +test_expect_success 'drop' ' > > Please, be more descriptive in the first argument of > test_expect_success. It's usually a good thing to say not only what the > test stresses but also what the expected behavior is. > > test_expect_success 'drop actually drops the line

Re: [PATCH/RFCv2 2/2] git rebase -i: warn about removed commits

2015-06-02 Thread Remi Galan Alfonso
Eric Sunshine writes: > Although the underlying behavior of "ignore" may be the same as the > default, you also want to check that higher-level machinery for > recognizing the "ignore" option is working correctly, which is why > checking "ignore" explicitly is a reasonable thing to do. Noted

Re: [PATCH/RFCv2 2/2] git rebase -i: warn about removed commits

2015-06-02 Thread Remi Galan Alfonso
Matthieu Moy writes: > Galan Rémi writes: > > > Check if commits were removed (i.e. a line was deleted) and print > > warnings or abort git rebase according to the value of the > > configuration variable rebase.checkLevel. > > checkLevel does not seem to be a good name, because it doesn't say >

Re: [PATCH/RFCv2 2/2] git rebase -i: warn about removed commits

2015-06-02 Thread Remi Galan Alfonso
Eric Sunshine writes: > > +rebase.checkLevel:: > > + If set to "warn", git rebase -i will print a warning if some > > + commits are removed (i.e. a line was deleted) or if some > > + commits appear more than one time (e.g. the same commit is > > + picked twice), however the

Re: [PATCH/RFCv2 1/2] git-rebase -i: add command "drop" to remove a commit

2015-06-02 Thread Remi Galan Alfonso
> Ideally, I think we should do a sanity check before starting the rebase, > and error out if we encounter an invalid command, a command that should > be followed by a valid sha1 and does not, ... > > But currently, we do the verification while applying commands, and I > don't think there's anythi

Re: [PATCH/RFCv2 1/2] git-rebase -i: add command "drop" to remove a commit

2015-06-01 Thread Remi Galan Alfonso
Junio C Hamano writes: > Is this sufficient? > If you are going to do something in 2/2 that relies on the format of > this line being correct (as opposed to "noop" or "#" that can have > any garbage on the remainder of the line), wouldn't you want to at > least check $sha1 is sensible? That's al

Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit

2015-05-29 Thread Remi Galan Alfonso
At first we wanted a clear indication about removing a commit in rebase -i. We didn't know about the noop command. - 'noop' does the same thing as 'drop' but for the user deleting a commit through 'noop' doesn't seem to be the proper way to use this command. Moreover 'noop' is not documented

Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits

2015-05-28 Thread Remi Galan Alfonso
Junio C Hamano writes: > I think there is a difference between (silently) accepting just to > be lenient and documenting and advocating mixed case uses. > > Personally, I'd rather not to see gratuitous flexibility to allow > the same thing spelled in 47 different ways for no good reason. It was

Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit

2015-05-27 Thread Remi Galan Alfonso
Thank you for reviewing the code. Johannes Schindelin writes: > Please note that you can already just comment-out the line if you need to > keep a visual trace. > > Alternatively, you can replace the `pick` command by `noop`. > > If you really need the `drop` command (with which I am not 100% >

Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits

2015-05-27 Thread Remi Galan Alfonso
Eric Sunshine writes: > Shouldn't this case also 'die' when rebase.checkLevel is "error"? And, > why doesn't the user get advice about configuring rebase.checkLevel in > this case? Stephen Kelly writes: > I sometimes duplicate commits deliberately if I want to split a commit in > two. Matthieu Moy

Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits

2015-05-27 Thread Remi Galan Alfonso
Thank you for reviewing the code. Eric Sunshine writes: > > + # To uppercase > > + checkLevel=$(echo "$checkLevel" | tr '[:lower:]' '[:upper:]') > > Is there precedence elsewhere for recognizing uppercase and lowercase > variants of config values? It seems to be commonly used when p