unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Trouble merging line-numbers branch into master
@ 2017-07-07 15:43 Kaushal Modi
  2017-07-07 15:51 ` Noam Postavsky
  2017-07-07 21:23 ` Alan Mackenzie
  0 siblings, 2 replies; 10+ messages in thread
From: Kaushal Modi @ 2017-07-07 15:43 UTC (permalink / raw)
  To: Emacs developers


[-- Attachment #1.1: Type: text/plain, Size: 1038 bytes --]

Hello,

I have having trouble merging the scratch/line-numbers branch into master.

I get one merge conflict in NEWS. That is fine; I resolved that locally.
But then further merging fails because of

  1 git … commit --
lisp/progmodes/cc-fonts.el:1251: space before tab in indent.
+   (c-major-mode-is 'c++-mode)
lisp/progmodes/cc-fonts.el:1252: space before tab in indent.
+   (save-excursion (c-back-over-member-initializers)))


You can see that there is white-space trapped between tabs (whitespace
mode) on lines 1251 and 1252:

[image: image.png]
(Look! I am making use of the new line-numbers feature to visually
communicate with people remotely :) )

So something is configured in git that blocks this merging is such mix of
white-space/tabs is seen? In any case, I removed those spaces, but still
cannot proceed with the merge. How can I move forward with the merge?

Also, shouldn't that commit have been blocked in the first place instead of
complaining about it when merging?
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 1677 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 35535 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-07 15:43 Trouble merging line-numbers branch into master Kaushal Modi
@ 2017-07-07 15:51 ` Noam Postavsky
  2017-07-07 18:08   ` Kaushal Modi
  2017-07-07 21:23 ` Alan Mackenzie
  1 sibling, 1 reply; 10+ messages in thread
From: Noam Postavsky @ 2017-07-07 15:51 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: Emacs developers

On Fri, Jul 7, 2017 at 11:43 AM, Kaushal Modi <kaushal.modi@gmail.com> wrote:
> Also, shouldn't that commit have been blocked in the first place instead of complaining about it when merging?

If you rerun './autogen git' in a checkout which includes [1:
e20ad44], the git hook should be updated such that it won't complain
about changes from merged commits anymore.

1: 2017-04-29 11:42:13 -0700 e20ad449deefa7470386bf99e05fd8c67227f678
  Allow bypassing of some checks when merging



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-07 15:51 ` Noam Postavsky
@ 2017-07-07 18:08   ` Kaushal Modi
  0 siblings, 0 replies; 10+ messages in thread
From: Kaushal Modi @ 2017-07-07 18:08 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On Fri, Jul 7, 2017 at 11:51 AM Noam Postavsky <
npostavs@users.sourceforge.net> wrote:

> If you rerun './autogen git' in a checkout which includes [1:
> e20ad44], the git hook should be updated such that it won't complain
> about changes from merged commits anymore.
>
> 1: 2017-04-29 11:42:13 -0700 e20ad449deefa7470386bf99e05fd8c67227f678
>   Allow bypassing of some checks when merging
>

Thanks!

Soon after I emailed, I was able to get past that problem by committing the
fix that removed those spaces in my local master and then restarting the
merge.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1011 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-07 15:43 Trouble merging line-numbers branch into master Kaushal Modi
  2017-07-07 15:51 ` Noam Postavsky
@ 2017-07-07 21:23 ` Alan Mackenzie
  2017-07-07 21:38   ` Stefan Monnier
  2017-07-07 21:41   ` Paul Eggert
  1 sibling, 2 replies; 10+ messages in thread
From: Alan Mackenzie @ 2017-07-07 21:23 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: Emacs developers

Hello, Kaushal

On Fri, Jul 07, 2017 at 15:43:24 +0000, Kaushal Modi wrote:
> Hello,

> I have having trouble merging the scratch/line-numbers branch into master.

> I get one merge conflict in NEWS. That is fine; I resolved that locally.
> But then further merging fails because of

>   1 git … commit --
> lisp/progmodes/cc-fonts.el:1251: space before tab in indent.
> +   (c-major-mode-is 'c++-mode)
> lisp/progmodes/cc-fonts.el:1252: space before tab in indent.
> +   (save-excursion (c-back-over-member-initializers)))


> You can see that there is white-space trapped between tabs (whitespace
> mode) on lines 1251 and 1252:

I've just counted up occurrences of <space><tab> in our elisp files, and
we've got around 2058 in 263 distinct files.  A lot of these appear to
have been where `comment-region' has inserted ";; " before tabs, but not
all of them.

Have you any idea what's special about those two <space><tab>s in
cc-fonts.el that git complained about them, but not the other 2056
occurrences?

> [image: image.png]
> (Look! I am making use of the new line-numbers feature to visually
> communicate with people remotely :) )

> So something is configured in git that blocks this merging is such mix of
> white-space/tabs is seen? In any case, I removed those spaces, but still
> cannot proceed with the merge. How can I move forward with the merge?

> Also, shouldn't that commit have been blocked in the first place instead of
> complaining about it when merging?

I think there's something more subtle going on here.

> -- 

> Kaushal Modi

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-07 21:23 ` Alan Mackenzie
@ 2017-07-07 21:38   ` Stefan Monnier
  2017-07-07 21:41   ` Paul Eggert
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2017-07-07 21:38 UTC (permalink / raw)
  To: emacs-devel

> Have you any idea what's special about those two <space><tab>s in
> cc-fonts.el that git complained about them, but not the other 2056
> occurrences?

These occurrences were in lines that have been touched somehow by
the merge: the check only looks at text lines that are modified.



        Stefan




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-07 21:23 ` Alan Mackenzie
  2017-07-07 21:38   ` Stefan Monnier
@ 2017-07-07 21:41   ` Paul Eggert
  2017-07-07 21:46     ` Kaushal Modi
  1 sibling, 1 reply; 10+ messages in thread
From: Paul Eggert @ 2017-07-07 21:41 UTC (permalink / raw)
  To: Alan Mackenzie, Kaushal Modi; +Cc: Emacs developers

Presumably the lines in question were changed in one branch or the other (not 
necessarily by messing with white space), and our git settings are suggesting 
that you omit the spaces before tabs while you're futzing with those lines.

As build-aux/git-hooks/pre-commit suggests, you can work around the problem 
temporarily with the shell command 'git config core.whitespace -trailing-space'. 
I don't recommend this setting in general, though.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-07 21:41   ` Paul Eggert
@ 2017-07-07 21:46     ` Kaushal Modi
  2017-07-08  8:04       ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Kaushal Modi @ 2017-07-07 21:46 UTC (permalink / raw)
  To: Paul Eggert, Alan Mackenzie, Eli Zaretskii; +Cc: Emacs developers

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

On Fri, Jul 7, 2017 at 5:41 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> Presumably the lines in question were changed in one branch or the other
> (not
> necessarily by messing with white space), and our git settings are
> suggesting
> that you omit the spaces before tabs while you're futzing with those lines.
>
> As build-aux/git-hooks/pre-commit suggests, you can work around the problem
> temporarily with the shell command 'git config core.whitespace
> -trailing-space'.
> I don't recommend this setting in general, though.
>

I found the error useful. So when line-numbers branch is eventually merged,
instead of ignoring/bypassing this error, I'd suggest that that
tab+space+tab combo be fixed and that whole thing be made to contain only
tabs or only spaces (my preference).

Slowly and gradually, as more merges and commits happen in future, all such
mixtures of tabs+spaces will get removed.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1352 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-07 21:46     ` Kaushal Modi
@ 2017-07-08  8:04       ` Eli Zaretskii
  2017-07-08 12:05         ` Noam Postavsky
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2017-07-08  8:04 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: acm, eggert, emacs-devel

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Fri, 07 Jul 2017 21:46:44 +0000
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> On Fri, Jul 7, 2017 at 5:41 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
> 
>  Presumably the lines in question were changed in one branch or the other (not
>  necessarily by messing with white space), and our git settings are suggesting
>  that you omit the spaces before tabs while you're futzing with those lines.
> 
>  As build-aux/git-hooks/pre-commit suggests, you can work around the problem
>  temporarily with the shell command 'git config core.whitespace -trailing-space'.
>  I don't recommend this setting in general, though.
> 
> I found the error useful. So when line-numbers branch is eventually merged, instead of ignoring/bypassing this
> error, I'd suggest that that tab+space+tab combo be fixed and that whole thing be made to contain only tabs
> or only spaces (my preference). 

I've just merged the branch, and didn't have any such problems.  In
the diffs I see no changes due to whitespace only, and the only merge
conflict I had, in NEWS, had nothing to do with whitespace and was
resolved without introducing any whitespace.

So I really don't understand what is this all about.  If SPC followed
by a TAB is detected when you merge, it was already there in master
before the merge, and the merge itself is not the problem.

In any case, if someone bumps into this issue, please do NOT fix
whitespace as part of very large commits (such as the one I just did),
because in such jumbo changesets it's very important to be able to
identify real changes, and too many whitespace changes get in the way.

> Slowly and gradually, as more merges and commits happen in future, all such mixtures of tabs+spaces will
> get removed.

They cannot be removed completely, because indentation doesn't always
end in a column whose number is an integral multiple of the tab width.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-08  8:04       ` Eli Zaretskii
@ 2017-07-08 12:05         ` Noam Postavsky
  2017-07-08 12:46           ` Kaushal Modi
  0 siblings, 1 reply; 10+ messages in thread
From: Noam Postavsky @ 2017-07-08 12:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Alan Mackenzie, Paul Eggert, Emacs developers, Kaushal Modi

On Sat, Jul 8, 2017 at 4:04 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> I've just merged the branch, and didn't have any such problems.  In
> the diffs I see no changes due to whitespace only, and the only merge
> conflict I had, in NEWS, had nothing to do with whitespace and was
> resolved without introducing any whitespace.
>
> So I really don't understand what is this all about.  If SPC followed
> by a TAB is detected when you merge, it was already there in master
> before the merge, and the merge itself is not the problem.

The old git hook checks *both* sides of the merge, so changes
introduced on the master branch after you branched off would also
trigger the error, but only if you happen to hit a conflict or
otherwise end up editing the merge commit message for some other
reason (e.g., using a dumb terminal which can't run $EDITOR).

The latter confusing condition is why the hook was changed to only
check the newly merged in changes, but you need to run './autogen git'
to update it in your local copy (I assume Kaushal had not done so,
hence the error).

>> Slowly and gradually, as more merges and commits happen in future, all such mixtures of tabs+spaces will
>> get removed.
>
> They cannot be removed completely, because indentation doesn't always
> end in a column whose number is an integral multiple of the tab width.

I believe git only complains about tab following spaces, not spaces
following tabs.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Trouble merging line-numbers branch into master
  2017-07-08 12:05         ` Noam Postavsky
@ 2017-07-08 12:46           ` Kaushal Modi
  0 siblings, 0 replies; 10+ messages in thread
From: Kaushal Modi @ 2017-07-08 12:46 UTC (permalink / raw)
  To: Noam Postavsky, Eli Zaretskii
  Cc: Alan Mackenzie, Paul Eggert, Emacs developers

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Sat, Jul 8, 2017, 8:05 AM Noam Postavsky <npostavs@users.sourceforge.net>
wrote:

>
> The latter confusing condition is why the hook was changed to only
> check the newly merged in changes, but you need to run './autogen git'
> to update it in your local copy (I assume Kaushal had not done so,
> hence the error).
>

I do run "./autogen.sh all" but only when the configure file is not present
(doing this via a wrapper script). So I definitely must have run that
before your earlier referenced commit [1]. I think I should instead
run "./autogen.sh git" too during each build now.

I believe git only complains about tab following spaces, not spaces
> following tabs.
>

Yes, this one is a weird case where spaces were trapped between 2 tabs.

>
[1]: 2017-04-29 11:42:13 -0700 e20ad449deefa7470386bf99e05fd8c67227f678

Allow bypassing of some checks when merging

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1822 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-07-08 12:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-07 15:43 Trouble merging line-numbers branch into master Kaushal Modi
2017-07-07 15:51 ` Noam Postavsky
2017-07-07 18:08   ` Kaushal Modi
2017-07-07 21:23 ` Alan Mackenzie
2017-07-07 21:38   ` Stefan Monnier
2017-07-07 21:41   ` Paul Eggert
2017-07-07 21:46     ` Kaushal Modi
2017-07-08  8:04       ` Eli Zaretskii
2017-07-08 12:05         ` Noam Postavsky
2017-07-08 12:46           ` Kaushal Modi

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).