[PATCH] push: hg should provide hint when no paths configured (issue3692)

[Mads Kiilerich]

On 02/11/2014 12:12 PM, Anurag Goel wrote:
> # HG changeset patch
> # User anuraggoel <anurag.dsps at gmail.com>
> # Date 1392116931 -19800
> #      Tue Feb 11 16:38:51 2014 +0530
> # Node ID 6bda034329f21f522b4bb2498bae4c84f3653569
> # Parent  fff0a71f8177be4ca634784d8b603c8e4d6c3601
> push: hg should provide hint when no paths configured (issue3692)
>
> Hint will be provided to the user if default path is not configured when user type "hg push" command.
> I also run the code on the test cases this time, all seems to work fine.

This patch is getting better!

> diff -r fff0a71f8177 -r 6bda034329f2 mercurial/commands.py
> --- a/mercurial/commands.py	Thu Feb 06 02:17:48 2014 +0100
> +++ b/mercurial/commands.py	Tue Feb 11 16:38:51 2014 +0530
> @@ -4711,7 +4711,16 @@
>       dest, branches = hg.parseurl(dest, opts.get('branch'))
>       ui.status(_('pushing to %s\n') % util.hidepassword(dest))
>       revs, checkout = hg.addbranchrevs(repo, repo, branches, opts.get('rev'))
> -    other = hg.peer(repo, opts, dest)
> +    if dest == "default-push":
> +        try:
> +            other = hg.peer(repo, opts, dest)
> +        except error.RepoError:
> +            error1 = (_("repository default-push not found!"))

Why the extra ()? The 1 suffix also seems odd. I would call it "msg" 
like it is one in at least one or place in that file ... or do like most 
other places and just let the Abort command span multiple lines.

> +            hint = _("see the \"path\" section in \"hg help config\"")
> +            raise util.Abort(error1, hint=hint)
> +    else:
> +        other = hg.peer(repo,opts,dest)

This line proves that you haven't run the test suite. Step 9 on 
http://mercurial.selenic.com/wiki/ContributingChanges .

Also, how about adding some test coverage of this? You already got the 
link to http://mercurial.selenic.com/wiki/WritingTests .

One important test case: having a default-path configuration that points 
at a non-existing location.

/Mads