Re: [PATCH v1 0/4] replace: add option to edit a Git object

2014-04-29 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Sat, Apr 26, 2014 at 10:00:53PM +0200, Christian Couder wrote:

 This patch series comes from what Peff sent in the following thread:
 
 http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528

 Thanks. As I recall, these were in pretty good shape, and I just read
 over them again and didn't see anything wrong.

 I added the following fixes:
 
 - add strbuf_release(result); in import_object(); this was suggested
   by Eric Sunshine
 - use MODE_LIST instead of MODE_DELETE if no arguments are passed; this
   makes the test suite pass
 - add --no-replace-objects when calling git cat-file in export_object();
   so that we edit the original object if an object is already replaced

 All sensible, I think.

 I am not happy with the fact that if the user doesn't modify the object when
 editing it, then a replace ref can still be created that points to the
 original object. I think something should be done to avoid that.

 Yeah, it should be easy to just hashcmp the sha1s after calling
 import_object. In fact, I think we can just erase any existing replace
 ref in that case (the user might have started with a replace ref and
 converted it _back_ to the original object, for example).

 Once that is fixed, I plan to add some tests and documentation, but I wanted
 first to let you know that I am looking at this.

 Great. Thanks for working on this.

 -Peff

Thanks.  In the meantime, I'll queue these as-is and push the result
out.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 0/4] replace: add option to edit a Git object

2014-04-28 Thread Jeff King
On Sat, Apr 26, 2014 at 10:00:53PM +0200, Christian Couder wrote:

 This patch series comes from what Peff sent in the following thread:
 
 http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528

Thanks. As I recall, these were in pretty good shape, and I just read
over them again and didn't see anything wrong.

 I added the following fixes:
 
 - add strbuf_release(result); in import_object(); this was suggested
   by Eric Sunshine
 - use MODE_LIST instead of MODE_DELETE if no arguments are passed; this
   makes the test suite pass
 - add --no-replace-objects when calling git cat-file in export_object();
   so that we edit the original object if an object is already replaced

All sensible, I think.

 I am not happy with the fact that if the user doesn't modify the object when
 editing it, then a replace ref can still be created that points to the
 original object. I think something should be done to avoid that.

Yeah, it should be easy to just hashcmp the sha1s after calling
import_object. In fact, I think we can just erase any existing replace
ref in that case (the user might have started with a replace ref and
converted it _back_ to the original object, for example).

 Once that is fixed, I plan to add some tests and documentation, but I wanted
 first to let you know that I am looking at this.

Great. Thanks for working on this.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v1 0/4] replace: add option to edit a Git object

2014-04-26 Thread Christian Couder
This patch series comes from what Peff sent in the following thread:

http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528

I added the following fixes:

- add strbuf_release(result); in import_object(); this was suggested
  by Eric Sunshine
- use MODE_LIST instead of MODE_DELETE if no arguments are passed; this
  makes the test suite pass
- add --no-replace-objects when calling git cat-file in export_object();
  so that we edit the original object if an object is already replaced

I am not happy with the fact that if the user doesn't modify the object when
editing it, then a replace ref can still be created that points to the
original object. I think something should be done to avoid that.

Once that is fixed, I plan to add some tests and documentation, but I wanted
first to let you know that I am looking at this.

Jeff King (4):
  replace: refactor command-mode determination
  replace: use OPT_CMDMODE to handle modes
  replace: factor object resolution out of replace_object
  replace: add --edit option

 builtin/replace.c | 189 --
 1 file changed, 154 insertions(+), 35 deletions(-)

-- 
1.9.1.636.g20d5f34

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html