This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2588

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

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

Download Raw Diff

Details

Reviewers

baymax

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

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMar 3 2018, 12:27 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

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

sangeet259 added a comment.Mar 3 2018, 12:41 PM

This comment was removed by sangeet259.

sangeet259 updated this revision to Diff 6536.Mar 4 2018, 12:04 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.

tom.prince added a subscriber: tom.prince.Mar 5 2018, 11:42 AM

@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 a subscriber: durin42.Mar 7 2018, 10:50 AM

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?

In D2588#44645, @sangeet259 wrote:

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.

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Jan 24 2020, 12:33 PM

Revision Contents
Changeset List

			Path	Packages
M			mercurial/fancyopts.py (10 lines)
M			tests/test-commit.t (18 lines)

Diff 6537

mercurial/fancyopts.py

	name = argmap[opt]			name = argmap[opt]
	obj = defmap[name]			obj = defmap[name]
	if obj._isboolopt():			if obj._isboolopt():
	state[name] = boolval			state[name] = boolval
	else:			else:
	def abort(s):			def abort(s):
	raise error.Abort(			raise error.Abort(
	_('invalid value %r for option %s, %s') % (val, opt, s))			_('invalid value %r for option %s, %s') % (val, opt, s))
				if name == 'message':
				pulkitUnsubmitted Not Done I think this is not the right place to have this hack. We should have this as a part of `hg commit` code rather. pulkit: I think this is not the right place to have this hack. We should have this as a part of `hg…
				yujaUnsubmitted Not Done 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. yuja: Right. Alternatively, we could add a subclass of customopt, but I don't know which would be…
				if state[name] != '':
				new_message = state[name] + "\n\n" + val
				state[name] = defmap[name].newstate(state[name], \
				new_message, abort)
				else:
				state[name] = defmap[name].newstate(state[name], val, abort)
				else :
	state[name] = defmap[name].newstate(state[name], val, abort)			state[name] = defmap[name].newstate(state[name], val, abort)

	# return unparsed args			# return unparsed args
	return args			return args

tests/test-commit.t

	foo			foo
	+foo			+foo
	$ hg log -T '{desc}\n' -r .			$ hg log -T '{desc}\n' -r .
	first line3			first line3
	second line			second line

	$ cd ..			$ cd ..

				Test that commit accpets multiple message (-m) flags

				$ hg init test4
				$ cd test4/
				$ echo a>>a
				$ hg commit -qAm "This is the first line" -m "This is the follwing line"
				$ hg log -v
				changeset: 0:5e33a0fb6989
				tag: tip
				user: test
				date: Thu Jan 01 00:00:00 1970 +0000
				files: a
				description:
				pulkitUnsubmitted Not Done starting with a newline seems awkward. pulkit: starting with a newline seems awkward.
				durin42Unsubmitted Not Done agreed durin42: agreed
				This is the first line

				This is the follwing line

				$ cd ..

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	6438		Mar 3 2018, 12:27 PM	★	★
Diff 2	6536		Mar 4 2018, 12:04 AM	★	★
Diff 3	6537		Mar 4 2018, 2:42 AM	★	★