bug#43117: [PATCH] Add .git-blame-ignore-revs file

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Emacs - Bugs mailing list
I made some style changes to em-hist.el and registered that commit in the new .git-blame-ignore-revs file to preserve blame data of other commits. The single-clause if statements frequently throw me off, so I think it's worth removing them now that there's a way to do so without ruining git blame sessions.

I signed an FSF copyright assignment about a year ago when I submitted something to the Emacs package Ivy. Attached are the files they sent me confirming successful registration. If I need to send something else instead, please let me know.

Thanks,
Brian
-- 
Sent with https://mailfence.com
Secure and private email

0001-eshell-em-hist-Style-and-indentation-cleanup.patch (43K) Download Attachment
0002-Add-.git-blame-ignore-revs.patch (1015 bytes) Download Attachment
Hsieh.pdf.asc (836 bytes) Download Attachment
Leung.pdf (199K) Download Attachment
Leung.pdf.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Robert Pluim
>>>>> On Sun, 30 Aug 2020 19:46:31 +0200 (CEST), Brian Leung via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <[hidden email]> said:

    Brian> I made some style changes to em-hist.el and registered that commit in
    Brian> the new .git-blame-ignore-revs file to preserve blame data of other
    Brian> commits. The single-clause if statements frequently throw me off, so I
    Brian> think it's worth removing them now that there's a way to do so without
    Brian> ruining git blame sessions.

    Brian> I signed an FSF copyright assignment about a year ago when I submitted
    Brian> something to the Emacs package Ivy. Attached are the files they sent
    Brian> me confirming successful registration. If I need to send something
    Brian> else instead, please let me know.

You've configured emacs to replace TAB with SPC. Please donʼt do that,
it makes it very hard to figure out what you've actually changed.

Robert



Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Dmitry Gutov
On 31.08.2020 13:16, Robert Pluim wrote:
> You've configured emacs to replace TAB with SPC.

That's what our .dir-locals.el does, doesn't it?



Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Dmitry Gutov
On 31.08.2020 23:34, Robert Pluim wrote:
>      Dmitry> On 31.08.2020 13:16, Robert Pluim wrote:
>      >> You've configured emacs to replace TAB with SPC.
>
>      Dmitry> That's what our .dir-locals.el does, doesn't it?
>
> Only for lines that you change. The patch has wholesale TAB->SPC
> replacement for lines which haven't otherwise been modified.

Right.

In that case, we should probably clarify that we don't usually
whitespace-only changes far from the code that's being changes.

Do we accept reindentation patches, though? Ones where the effective
indentation does change.



Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Robert Pluim
>>>>> On Mon, 31 Aug 2020 23:45:55 +0300, Dmitry Gutov <[hidden email]> said:

    Dmitry> On 31.08.2020 23:34, Robert Pluim wrote:
    Dmitry> On 31.08.2020 13:16, Robert Pluim wrote:
    >> >> You've configured emacs to replace TAB with SPC.
    Dmitry> That's what our .dir-locals.el does, doesn't it?
    >> Only for lines that you change. The patch has wholesale TAB->SPC
    >> replacement for lines which haven't otherwise been modified.

    Dmitry> Right.

    Dmitry> In that case, we should probably clarify that we don't usually
    Dmitry> whitespace-only changes far from the code that's being changes.

    Dmitry> Do we accept reindentation patches, though? Ones where the effective
    Dmitry> indentation does change.

Didnʼt we just have this thread? :-)

Pure re-indentation patches I thought were not acceptable unless
accompanied by an actual code change near the re-indented lines.

Robert



Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Emacs - Bugs mailing list
OK, please see attached. Right now there are no extraneous whitespace changes, and the only changes I've made change if -> when in the eshell files.

> Pure re-indentation patches I thought were not acceptable unless
> accompanied by an actual code change near the re-indented lines.

With a .git-blame-ignore file of some sort, shouldn't this not be a hindrance?

> ----------------------------------------
> From: Dmitry Gutov <[hidden email]>
> Sent: Mon Aug 31 23:03:23 CEST 2020
> To: Robert Pluim <[hidden email]>
> Cc: Brian Leung <[hidden email]>, <[hidden email]>
> Subject: Re: bug#43117: [PATCH] Add .git-blame-ignore-revs file
>
>
> On 31.08.2020 23:57, Robert Pluim wrote:
>
> >      Dmitry> Do we accept reindentation patches, though? Ones where the effective
> >      Dmitry> indentation does change.
> >
> > Didnʼt we just have this thread? :-)
>
> I might have missed it. :-(
>
> > Pure re-indentation patches I thought were not acceptable unless
> > accompanied by an actual code change near the re-indented lines.
>
> Okay.
>
> Brian, could you re-submit patches without extra re-indentation?

-- 
Sent with https://mailfence.com
Secure and private email

0001-eshell-Replace-single-clause-if-statements-with-when.patch (139K) Download Attachment
0002-Add-.git-blame-ignore-revs.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Eli Zaretskii
> Cc: [hidden email]
> Date: Sun, 13 Sep 2020 02:42:23 +0200 (CEST)
> From: Brian Leung via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[hidden email]>
>
> OK, please see attached. Right now there are no extraneous whitespace changes, and the only changes I've made change if -> when in the eshell files.
> [...]
> Subject: [PATCH 1/2] eshell: Replace single-clause if statements with whens

I'm not sure I understand the rationale: why would we want to replace
all 'if's with 'when's?  Was that discussed, here or elsewhere? if so,
could someone point me to that discussion?

Thanks.



Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Lars Ingebrigtsen
Eli Zaretskii <[hidden email]> writes:

> Is it just me? do others think such changes are a good idea?

I hate one-form-`if' more than anybody, but I don't think these purely
stylistic rewrites are a good idea, as it means that "git blame" and
vc-region-history gets less useful, and applying older patches fails
more often.

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



Reply | Threaded
Open this post in threaded view
|

bug#43117: [PATCH] Add .git-blame-ignore-revs file

Emacs - Bugs mailing list
> I don't think these purely
> stylistic rewrites are a good idea, as it means that "git blame" and
> vc-region-history gets less useful, and applying older patches fails
> more often.

OK, I won't submit that commit then. The other commit I had introduced a .git-blame-ignore-revs file, which allows certain commits to be ignored during git-blame sessions. I've modified that commit accordingly and attached. Would that be of use?

My FSF papers are attached.

> ----------------------------------------
> From: Lars Ingebrigtsen <[hidden email]>
> Sent: Sun Sep 13 19:57:25 CEST 2020
> To: Eli Zaretskii <[hidden email]>
> Cc: Brian Leung <[hidden email]>, <[hidden email]>, <[hidden email]>, <[hidden email]>
> Subject: Re: bug#43117: [PATCH] Add .git-blame-ignore-revs file
>
>
> Eli Zaretskii <[hidden email]> writes:
>
> > Is it just me? do others think such changes are a good idea?
>
> I hate one-form-`if' more than anybody, but I don't think these purely
> stylistic rewrites are a good idea, as it means that "git blame" and
> vc-region-history gets less useful, and applying older patches fails
> more often.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Sent with https://mailfence.com
Secure and private email

0001-Add-.git-blame-ignore-revs-file.patch (1K) Download Attachment
leung.tar.gz (198K) Download Attachment