Patchwork [1,of,7] share: use context manager or utility function to write file

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 13, 2018, 5:02 a.m.
Message ID <2eeaf96c20fce19c8edc.1515819772@mimosa>
Download mbox | patch
Permalink /patch/26715/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 13, 2018, 5:02 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1515817396 -32400
#      Sat Jan 13 13:23:16 2018 +0900
# Node ID 2eeaf96c20fce19c8edccf4936aceee4ce651de9
# Parent  991f0be9dc39c402d63a4a8f19cde052095c4689
share: use context manager or utility function to write file
Gregory Szorc - Jan. 14, 2018, 7:24 a.m.
On Fri, Jan 12, 2018 at 9:02 PM, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1515817396 -32400
> #      Sat Jan 13 13:23:16 2018 +0900
> # Node ID 2eeaf96c20fce19c8edccf4936aceee4ce651de9
> # Parent  991f0be9dc39c402d63a4a8f19cde052095c4689
> share: use context manager or utility function to write file
>

Queued this series.

I'm personally not a huge fan of the helper APIs in the vfs/io layer: I
would prefer to stick to the primitive I/O APIs and e.g. open files with
context managers in append mode. But it's not a big deal.

I do support ditching text mode. But for many of these uses cases, all
written data is ASCII, so encoding shouldn't matter. But opening files in
binary mode consistently throughout the code base makes sense. The change
feels strictly better.


>
> diff --git a/mercurial/hg.py b/mercurial/hg.py
> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -307,16 +307,13 @@ def postshare(sourcerepo, destrepo, book
>      """
>      default = defaultpath or sourcerepo.ui.config('paths', 'default')
>      if default:
> -        fp = destrepo.vfs("hgrc", "w", text=True)
> -        fp.write("[paths]\n")
> -        fp.write("default = %s\n" % default)
> -        fp.close()
> +        with destrepo.vfs("hgrc", "w", text=True) as fp:
> +            fp.write("[paths]\n")
> +            fp.write("default = %s\n" % default)
>
>      with destrepo.wlock():
>          if bookmarks:
> -            fp = destrepo.vfs('shared', 'w')
> -            fp.write(sharedbookmarks + '\n')
> -            fp.close()
> +            destrepo.vfs.write('shared', sharedbookmarks + '\n')
>
>  def _postshareupdate(repo, update, checkout=None):
>      """Maybe perform a working directory update after a shared repo is
> created.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - Jan. 14, 2018, 8:25 a.m.
On Sat, 13 Jan 2018 23:24:11 -0800, Gregory Szorc wrote:
> On Fri, Jan 12, 2018 at 9:02 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1515817396 -32400
> > #      Sat Jan 13 13:23:16 2018 +0900
> > # Node ID 2eeaf96c20fce19c8edccf4936aceee4ce651de9
> > # Parent  991f0be9dc39c402d63a4a8f19cde052095c4689
> > share: use context manager or utility function to write file
> >
> 
> Queued this series.
> 
> I'm personally not a huge fan of the helper APIs in the vfs/io layer:

Neither am I, at least for write/append operation. But I don't have strong
preference.

Patch

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -307,16 +307,13 @@  def postshare(sourcerepo, destrepo, book
     """
     default = defaultpath or sourcerepo.ui.config('paths', 'default')
     if default:
-        fp = destrepo.vfs("hgrc", "w", text=True)
-        fp.write("[paths]\n")
-        fp.write("default = %s\n" % default)
-        fp.close()
+        with destrepo.vfs("hgrc", "w", text=True) as fp:
+            fp.write("[paths]\n")
+            fp.write("default = %s\n" % default)
 
     with destrepo.wlock():
         if bookmarks:
-            fp = destrepo.vfs('shared', 'w')
-            fp.write(sharedbookmarks + '\n')
-            fp.close()
+            destrepo.vfs.write('shared', sharedbookmarks + '\n')
 
 def _postshareupdate(repo, update, checkout=None):
     """Maybe perform a working directory update after a shared repo is created.