Page MenuHomePhabricator

commit: adds multiline commit message support(issue5616)
Needs ReviewPublic

Authored by sangeet259 on Mar 3 2018, 12:27 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

The earlier functionality used to ignore the all but last -m flag's value.
It used to store the arguments information in a state dict and then overwrites each
subsequent value of -m in the 'message' key. This patch intercepts the message flag and
checks if it is already empty. In case it is not empty, add the current value of -m flag to
the message key with a leading '\n' character. This makes every subsequent passed -m flag
as the new line message of the commit.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

sangeet259 created this revision.Mar 3 2018, 12:27 PM
jeffpc added a subscriber: jeffpc.Mar 3 2018, 12:38 PM

FWIW, this is slightly different behavior from what git does. There, each -m is added as a separate *paragraph*. IOW,

$ git commit -m "first" -m "second"

creates:

first

second
This comment was removed by sangeet259.
sangeet259 updated this revision to Diff 6536.Mar 4 2018, 12:04 AM
sangeet259 added a comment.EditedMar 4 2018, 12:56 AM

@jeffpc That means two new lines. Shall I edit this patch to effect that?

sangeet259 updated this revision to Diff 6537.Mar 4 2018, 2:42 AM
pulkit added a subscriber: pulkit.Mar 4 2018, 11:44 AM
pulkit added inline comments.
mercurial/fancyopts.py
365

I think this is not the right place to have this hack. We should have this as a part of hg commit code rather.

tests/test-commit.t
847

starting with a newline seems awkward.

yuja added a subscriber: yuja.Mar 4 2018, 12:24 PM
yuja added inline comments.
mercurial/fancyopts.py
365

Right. Alternatively, we could add a subclass of customopt, but I don't know which
would be nicer.

FWIW, I'm kinda -1 on this feature, but I have no strong opinion and Git is the
current standard.

@pulkit Yeah, this rather a not so elegant hack. How can it be parsed in hg commit?
As the first message passed is overwritten by the second one at this step only. :/

@yuja I think maybe subclassing the customopt may be a fair choice.

durin42 added inline comments.
tests/test-commit.t
847

agreed

Since the current code just overwrites message each time with the newer. What can be done to avoid losing the earlier message values?

Since the current code just overwrites message each time with the newer. What can be done to avoid losing the earlier message values?

You should look how we handle multiple --rev flags. That will help you in this case.