This is an archive of the discontinued Mercurial Phabricator instance.

Differential D454

releasenotes: view admonition titles using -l flag
ClosedPublic

Authored by rishabhmadan96 on Aug 20 2017, 12:33 PM.

Details

Reviewers
pulkit
yuja
Group Reviewers
hg-reviewers
Commits
rHG2a37459aedf2: releasenotes: view admonition titles using -l flag
Summary

Since this extension is fairly new for almost all the contributors, remembering
the admonition (with titles) is difficult. The list (-l) flag provides
a list of all the active admonitions along with titles.

For usage, hg releasenotes -l returns the list.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit requested changes to this revision.Aug 21 2017, 7:55 AM
pulkit added a subscriber: pulkit.

The patch in general looks good to me. I have some inline comments.

hgext/releasenotes.py
278

Generally we add a '_' before an internal function. Since you need to resend because you need to rebase, can you change the function name to _getadmonitionlist. Also it will be extremely great if you send a patch documenting the functions in the releasenotes.py file.

475

I am not sure whether -a is the best choice of flag. Maybe we can have -l and --list.

Also the help can be improved to "list the available admonitions with their title".

tests/test-releasenotes-formatting.t
409–422

The above lines were added in D368 which is not yet pushed. So can you rebase this patch and resend so that it can be pushed independently.

This revision now requires changes to proceed.Aug 21 2017, 7:55 AM
rishabhmadan96 added inline comments.Aug 21 2017, 9:04 AM
hgext/releasenotes.py
475

--list sounds good to me. I'll make the changes.

rishabhmadan96 edited edge metadata.Aug 21 2017, 9:11 AM
rishabhmadan96 edited the summary of this revision. (Show Details)
rishabhmadan96 retitled this revision from releasenotes: view admonition titles using -a flag to releasenotes: view admonition titles using -l flag.
rishabhmadan96 updated this revision to Diff 1123.
rishabhmadan96 edited the summary of this revision. (Show Details)Aug 21 2017, 9:15 AM
yuja requested changes to this revision.Aug 27 2017, 6:03 AM
yuja added a subscriber: yuja.
yuja added inline comments.
hgext/releasenotes.py
247

Perhaps this can be just for section in sections since
releasenotessections implements __iter__.

248

Delete _(), which is the marker for strings to be translated.

"%s: %s\n" % ... should be better for python 3 compatibility.

558

merge error?

This revision now requires changes to proceed.Aug 27 2017, 6:03 AM
pulkit requested changes to this revision.Aug 27 2017, 5:15 PM

@rishabhmadan96 Also make sure a user cannot pass any other flag with the --list flag.

rishabhmadan96 added a comment.Aug 27 2017, 6:37 PM
In D454#8571, @pulkit wrote:

@rishabhmadan96 Also make sure a user cannot pass any other flag with the --list flag.

I'm using return while calling the function. Wouldn't that be enough for this?

rishabhmadan96 edited edge metadata.Aug 27 2017, 6:44 PM
rishabhmadan96 edited the summary of this revision. (Show Details)
rishabhmadan96 updated this revision to Diff 1331.
Closed by commit rHG2a37459aedf2: releasenotes: view admonition titles using -l flag (authored by rishabhmadan96). · Explain WhyAug 28 2017, 10:40 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 1352

hgext/releasenotes.py

raise error.Abort(_(".hgreleasenotes file \'%s\' not found") % raise error.Abort(_(".hgreleasenotes file \'%s\' not found") %
repo.pathto(f)) repo.pathto(f))
if '.hgreleasenotes' in ctx: if '.hgreleasenotes' in ctx:
read('.hgreleasenotes') read('.hgreleasenotes')
return p['sections'] return p['sections']
def checkadmonitions(ui, repo, directives, revs): def checkadmonitions(ui, repo, directives, revs):
""" """
yujaUnsubmitted
Not Done

Perhaps this can be just for section in sections since
releasenotessections implements __iter__.

yuja: Perhaps this can be just `for section in sections` since `releasenotessections` implements…
Checks the commit messages for admonitions and their validity. Checks the commit messages for admonitions and their validity.
yujaUnsubmitted
Not Done

Delete _(), which is the marker for strings to be translated.

"%s: %s\n" % ... should be better for python 3 compatibility.

yuja: Delete `_()`, which is the marker for strings to be translated. `"%s: %s\n" % ...` should be…
.. abcd:: .. abcd::
First paragraph under this admonition First paragraph under this admonition
For this commit message, using `hg releasenotes -r . --check` For this commit message, using `hg releasenotes -r . --check`
returns: Invalid admonition 'abcd' present in changeset 3ea92981e103 returns: Invalid admonition 'abcd' present in changeset 3ea92981e103
similar = [s for s in directives if sim(s) > 0.6] similar = [s for s in directives if sim(s) > 0.6]
if len(similar) == 1: if len(similar) == 1:
ui.write(_("(did you mean %s?)\n") % similar[0]) ui.write(_("(did you mean %s?)\n") % similar[0])
elif similar: elif similar:
ss = ", ".join(sorted(similar)) ss = ", ".join(sorted(similar))
ui.write(_("(did you mean one of %s?)\n") % ss) ui.write(_("(did you mean one of %s?)\n") % ss)
def _getadmonitionlist(ui, sections):
pulkitUnsubmitted
Not Done

Generally we add a '_' before an internal function. Since you need to resend because you need to rebase, can you change the function name to _getadmonitionlist. Also it will be extremely great if you send a patch documenting the functions in the releasenotes.py file.

pulkit: Generally we add a '_' before an internal function. Since you need to resend because you need…
for section in sections:
ui.write("%s: %s\n" % (section[0], section[1]))
def parsenotesfromrevisions(repo, directives, revs): def parsenotesfromrevisions(repo, directives, revs):
notes = parsedreleasenotes() notes = parsedreleasenotes()
for rev in revs: for rev in revs:
ctx = repo[rev] ctx = repo[rev]
blocks, pruned = minirst.parse(ctx.description(), blocks, pruned = minirst.parse(ctx.description(),
admonitions=directives) admonitions=directives)
if lines and lines[-1]: if lines and lines[-1]:
lines.append('') lines.append('')
return '\n'.join(lines) return '\n'.join(lines)
@command('releasenotes', @command('releasenotes',
[('r', 'rev', '', _('revisions to process for release notes'), _('REV')), [('r', 'rev', '', _('revisions to process for release notes'), _('REV')),
('c', 'check', False, _('checks for validity of admonitions (if any)'), ('c', 'check', False, _('checks for validity of admonitions (if any)'),
_('REV'))], _('REV')),
('l', 'list', False, _('list the available admonitions with their title'),
pulkitUnsubmitted
Not Done

I am not sure whether -a is the best choice of flag. Maybe we can have -l and --list.

Also the help can be improved to "list the available admonitions with their title".

pulkit: I am not sure whether `-a` is the best choice of flag. Maybe we can have `-l` and `--list`.
rishabhmadan96AuthorUnsubmitted
Not Done

--list sounds good to me. I'll make the changes.

rishabhmadan96: `--list` sounds good to me. I'll make the changes.
None)],
_('hg releasenotes [-r REV] [-c] FILE')) _('hg releasenotes [-r REV] [-c] FILE'))
def releasenotes(ui, repo, file_=None, **opts): def releasenotes(ui, repo, file_=None, **opts):
"""parse release notes from commit messages into an output file """parse release notes from commit messages into an output file
Given an output file and set of revisions, this command will parse commit Given an output file and set of revisions, this command will parse commit
messages for release notes then add them to the output file. messages for release notes then add them to the output file.
Release notes are defined in commit messages as ReStructuredText Release notes are defined in commit messages as ReStructuredText
greater than a threshold. greater than a threshold.
This means that the release notes file can be updated independently from This means that the release notes file can be updated independently from
this command and changes should not be lost when running this command on this command and changes should not be lost when running this command on
that file. A particular use case for this is to tweak the wording of a that file. A particular use case for this is to tweak the wording of a
release note after it has been added to the release notes file. release note after it has been added to the release notes file.
""" """
sections = releasenotessections(ui, repo) sections = releasenotessections(ui, repo)
rev = opts.get('rev') if opts.get('list'):
return _getadmonitionlist(ui, sections)
rev = opts.get('rev')
yujaUnsubmitted
Not Done

merge error?

yuja: merge error?
revs = scmutil.revrange(repo, [rev or 'not public()']) revs = scmutil.revrange(repo, [rev or 'not public()'])
if opts.get('check'): if opts.get('check'):
return checkadmonitions(ui, repo, sections.names(), revs) return checkadmonitions(ui, repo, sections.names(), revs)
incoming = parsenotesfromrevisions(repo, sections.names(), revs) incoming = parsenotesfromrevisions(repo, sections.names(), revs)
try: try:
with open(file_, 'rb') as fh: with open(file_, 'rb') as fh:

tests/test-releasenotes-formatting.t

$ hg -q commit -A -l - << EOF $ hg -q commit -A -l - << EOF
> .. fixes:: > .. fixes::
> >
> First paragraph under this admonition. > First paragraph under this admonition.
> EOF > EOF
$ hg releasenotes -r . -c $ hg releasenotes -r . -c
Invalid admonition 'fixes' present in changeset 0e7130d2705c Invalid admonition 'fixes' present in changeset 0e7130d2705c
(did you mean fix?) (did you mean fix?)
$ cd ..
Usage of --list flag
$ hg init relnotes-list
$ cd relnotes-list
$ hg releasenotes -l
feature: New Features
bc: Backwards Compatibility Changes
fix: Bug Fixes
perf: Performance Improvements
api: API Changes
pulkitUnsubmitted
Not Done

The above lines were added in D368 which is not yet pushed. So can you rebase this patch and resend so that it can be pushed independently.

pulkit: The above lines were added in D368 which is not yet pushed. So can you rebase this patch and…