Re: [PATCH 02 of 10 shelve-ext] shelve: add an ability to write json to a new type of shelve files

[timeless]

We should probably have check-code complain about it and explain why
we don't want it.
Self-documenting code is our friend.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 02 of 10 shelve-ext] shelve: add an ability to write json to a new type of shelve files

[Yuya Nishihara]

On Tue, 29 Nov 2016 15:55:06 +, Jun Wu wrote:
> Excerpts from Kostia Balytskyi's message of 2016-11-29 07:22:56 -0800:
> > # HG changeset patch
> > # User Kostia Balytskyi 
> > # Date 1480426193 28800
> > #  Tue Nov 29 05:29:53 2016 -0800
> > # Node ID 37119e028c699d9fabd220086e08c754827e709f
> > # Parent  f6f0ab3f7b0ea0e05cfdcd7afd4994ea21988fd9
> > shelve: add an ability to write json to a new type of shelve files
> > 
> > Obsolescense-based shelve only needs metadata stored in .hg/shelved
> > and I think that this metadata should be stored in json for
> > potential extensibility purposes. JSON is not critical here, but
> > I want to avoid storing it in an unstructured text file where
> > order of lines determines their semantical meanings (as now
> > happens in .hg/shelvedstate. .hg/rebasestate and I suspect other
> > state files as well).
> > 
> > If we want, we can refactor it to something else later.
> > 
> > diff --git a/hgext/shelve.py b/hgext/shelve.py
> > --- a/hgext/shelve.py
> > +++ b/hgext/shelve.py
> > @@ -25,6 +25,7 @@ from __future__ import absolute_import
> >  import collections
> >  import errno
> >  import itertools
> > +import json
> 
> I think we avoid using "import json" for some reason (encoding?). There is
> no "import json" in the code base yet.

Yeah, JSON is troublesome because every string must be a valid unicode. If
all strings are known to be ascii or utf-8 for instance, we could use JSON,
but we still have to convert unicode objects back to str.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 02 of 10 shelve-ext] shelve: add an ability to write json to a new type of shelve files

[Kostia Balytskyi]

On 11/29/16, 3:55 PM, "Jun Wu"  wrote:

Excerpts from Kostia Balytskyi's message of 2016-11-29 07:22:56 -0800:
> # HG changeset patch
> # User Kostia Balytskyi 
> # Date 1480426193 28800
> #  Tue Nov 29 05:29:53 2016 -0800
> # Node ID 37119e028c699d9fabd220086e08c754827e709f
> # Parent  f6f0ab3f7b0ea0e05cfdcd7afd4994ea21988fd9
> shelve: add an ability to write json to a new type of shelve files
> 
> Obsolescense-based shelve only needs metadata stored in .hg/shelved
> and I think that this metadata should be stored in json for
> potential extensibility purposes. JSON is not critical here, but
> I want to avoid storing it in an unstructured text file where
> order of lines determines their semantical meanings (as now
> happens in .hg/shelvedstate. .hg/rebasestate and I suspect other
> state files as well).
> 
> If we want, we can refactor it to something else later.
> 
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -25,6 +25,7 @@ from __future__ import absolute_import
>  import collections
>  import errno
>  import itertools
> +import json

I think we avoid using "import json" for some reason (encoding?). There is
no "import json" in the code base yet.

>  from mercurial.i18n import _
>  from mercurial import (
> @@ -62,7 +63,7 @@ testedwith = 'ships-with-hg-core'
>  
>  backupdir = 'shelve-backup'
>  shelvedir = 'shelved'
> -shelvefileextensions = ['hg', 'patch']
> +shelvefileextensions = ['hg', 'patch', 'oshelve']
>  # universal extension is present in all types of shelves
>  patchextension = 'patch'
>  
> @@ -153,6 +154,26 @@ class shelvedfile(object):
>  bundle2.writebundle(self.ui, cg, self.fname, btype, self.vfs,
>  compression=compression)
>  
> +def writejson(self, jsn):
> +fp = self.opener('wb')
> +try:
> +fp.write(json.dumps(jsn))
> +finally:
> +fp.close()
> +
> +def readjson(self):
> +fp = self.opener()
> +contents = None
> +try:
> +contents = fp.read()
> +finally:
> +fp.close()
> +try:
> +jsn = json.loads(contents)
> +except (TypeError, ValueError):
> +raise error.abort(_('could not read obsolescense-based 
shelve'))

error.Abort

The method does not seem to be related to "obsolescense" just from the name
and logic. So the error message would be inaccurate if callers use
"writejson" to write other things.

Given the fact "json" is a too generic name and we may replace it using
other (lightweight) format in the future, it may be better to rename those
methods to something more specific, and avoid exposing the "json" format
to the caller, like:

def writeobsinfo(self, node):


def readobsinfo(self):

Renaming it makes sense, I will do that in a following patch. I did not get 
what to do with json though: should I try to replace it with some other format 
right away or is it fine to leave json for now?

> +return jsn
> +
>  class shelvedstate(object):
>  """Handle persistence during unshelving operations.
>  


___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 02 of 10 shelve-ext] shelve: add an ability to write json to a new type of shelve files

[Jun Wu]

Excerpts from Kostia Balytskyi's message of 2016-11-29 07:22:56 -0800:
> # HG changeset patch
> # User Kostia Balytskyi 
> # Date 1480426193 28800
> #  Tue Nov 29 05:29:53 2016 -0800
> # Node ID 37119e028c699d9fabd220086e08c754827e709f
> # Parent  f6f0ab3f7b0ea0e05cfdcd7afd4994ea21988fd9
> shelve: add an ability to write json to a new type of shelve files
> 
> Obsolescense-based shelve only needs metadata stored in .hg/shelved
> and I think that this metadata should be stored in json for
> potential extensibility purposes. JSON is not critical here, but
> I want to avoid storing it in an unstructured text file where
> order of lines determines their semantical meanings (as now
> happens in .hg/shelvedstate. .hg/rebasestate and I suspect other
> state files as well).
> 
> If we want, we can refactor it to something else later.
> 
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -25,6 +25,7 @@ from __future__ import absolute_import
>  import collections
>  import errno
>  import itertools
> +import json

I think we avoid using "import json" for some reason (encoding?). There is
no "import json" in the code base yet.

>  from mercurial.i18n import _
>  from mercurial import (
> @@ -62,7 +63,7 @@ testedwith = 'ships-with-hg-core'
>  
>  backupdir = 'shelve-backup'
>  shelvedir = 'shelved'
> -shelvefileextensions = ['hg', 'patch']
> +shelvefileextensions = ['hg', 'patch', 'oshelve']
>  # universal extension is present in all types of shelves
>  patchextension = 'patch'
>  
> @@ -153,6 +154,26 @@ class shelvedfile(object):
>  bundle2.writebundle(self.ui, cg, self.fname, btype, self.vfs,
>  compression=compression)
>  
> +def writejson(self, jsn):
> +fp = self.opener('wb')
> +try:
> +fp.write(json.dumps(jsn))
> +finally:
> +fp.close()
> +
> +def readjson(self):
> +fp = self.opener()
> +contents = None
> +try:
> +contents = fp.read()
> +finally:
> +fp.close()
> +try:
> +jsn = json.loads(contents)
> +except (TypeError, ValueError):
> +raise error.abort(_('could not read obsolescense-based shelve'))

error.Abort

The method does not seem to be related to "obsolescense" just from the name
and logic. So the error message would be inaccurate if callers use
"writejson" to write other things.

Given the fact "json" is a too generic name and we may replace it using
other (lightweight) format in the future, it may be better to rename those
methods to something more specific, and avoid exposing the "json" format
to the caller, like:

def writeobsinfo(self, node):


def readobsinfo(self):


> +return jsn
> +
>  class shelvedstate(object):
>  """Handle persistence during unshelving operations.
>  
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 02 of 10 shelve-ext] shelve: add an ability to write json to a new type of shelve files

[Kostia Balytskyi]

# HG changeset patch
# User Kostia Balytskyi 
# Date 1480426193 28800
#  Tue Nov 29 05:29:53 2016 -0800
# Node ID 37119e028c699d9fabd220086e08c754827e709f
# Parent  f6f0ab3f7b0ea0e05cfdcd7afd4994ea21988fd9
shelve: add an ability to write json to a new type of shelve files

Obsolescense-based shelve only needs metadata stored in .hg/shelved
and I think that this metadata should be stored in json for
potential extensibility purposes. JSON is not critical here, but
I want to avoid storing it in an unstructured text file where
order of lines determines their semantical meanings (as now
happens in .hg/shelvedstate. .hg/rebasestate and I suspect other
state files as well).

If we want, we can refactor it to something else later.

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -25,6 +25,7 @@ from __future__ import absolute_import
 import collections
 import errno
 import itertools
+import json
 
 from mercurial.i18n import _
 from mercurial import (
@@ -62,7 +63,7 @@ testedwith = 'ships-with-hg-core'
 
 backupdir = 'shelve-backup'
 shelvedir = 'shelved'
-shelvefileextensions = ['hg', 'patch']
+shelvefileextensions = ['hg', 'patch', 'oshelve']
 # universal extension is present in all types of shelves
 patchextension = 'patch'
 
@@ -153,6 +154,26 @@ class shelvedfile(object):
 bundle2.writebundle(self.ui, cg, self.fname, btype, self.vfs,
 compression=compression)
 
+def writejson(self, jsn):
+fp = self.opener('wb')
+try:
+fp.write(json.dumps(jsn))
+finally:
+fp.close()
+
+def readjson(self):
+fp = self.opener()
+contents = None
+try:
+contents = fp.read()
+finally:
+fp.close()
+try:
+jsn = json.loads(contents)
+except (TypeError, ValueError):
+raise error.abort(_('could not read obsolescense-based shelve'))
+return jsn
+
 class shelvedstate(object):
 """Handle persistence during unshelving operations.
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel