Re: PATCH: git-p4 optional handling of RCS keywords

Re: PATCH: git-p4 optional handling of RCS keywords

[Junio C Hamano]

dhruva <dhruva@ymail.com> writes:

> @@ -975,10 +978,11 @@ class P4Sync(Command):
>                  sys.stderr.write("p4 print fails with: %s\n" % repr(stat))
>                  continue
>
> -            if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
> -                text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
> -            elif stat['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'binary+k'):
> -                text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$',r'$\1$', text)
> +            if kwstrip:
> +                if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
> +                    text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
> +                elif stat['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'binary+k'):
> +                    text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$',r'$\1$', text)

A style tip.

It makes it easier to convince others that you didn't screw up in the
conversion if you cascade the code this way instead:

-	if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
+	if not kwstrip:
+		pass
+	elif stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
 		text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
 	elif stat['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'binary+k'):
 		text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|...

This technique not just only makes the patch smaller and easier to review,
it also makes the result less deeply nested and easier to read as well.

Re: PATCH: git-p4 optional handling of RCS keywords

[Simon Hausmann]

[-- Attachment #1: Type: text/plain, Size: 1929 bytes --]

On Wednesday 10 September 2008 08:29:56 dhruva wrote:
> Hi,
>  Incorporated the style tip, sure makes it look cleaner.
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 2216cac..3e72e43 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -16,6 +16,9 @@ from sets import Set;
>
>  verbose = False
>
> +# Handling of RCS keyowrds. To ensure backward compatibility, the default
> +# is to strip keywords. Default behavior is controlled here
> +kwstrip = True
>
>  def p4_build_cmd(cmd):
>      """Build a suitable p4 command line.
> @@ -975,7 +978,9 @@ class P4Sync(Command):
>                  sys.stderr.write("p4 print fails with: %s\n" % repr(stat))
>                  continue
>
> -            if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
> +            if not kwstrip:
> +               pass
> +            elif stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
>                  text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
>              elif stat['type'] in ('text+k', 'ktext', 'kxtext',
> 'unicode+k', 'binary+k'): text =
> re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$',r
>'$\1$', text) @@ -1850,6 +1855,16 @@ def main():
>          (cmd, args) = parser.parse_args(sys.argv[2:], cmd);
>      global verbose
>      verbose = cmd.verbose
> +
> +    global kwstrip
> +    kwval = gitConfig("git-p4.kwstrip")
> +    if len(kwval) > 0:
> +       kwval = kwval.lower();
> +        if "false" == kwval:
> +            kwstrip = False
> +        elif "true" == kwval:
> +            kwstrip = True

I have another style nitpick, sorry :). Please use "kwval == "false" instead 
of the other way around.

Otherwise your patch looks good to me, I think it's a very good option to add. 
Please resend with commit message so that Junio can include it.


Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PATCH: git-p4 optional handling of RCS keywords

[dhruva]

Hello,


Commit message: Modifying RCS keywords prevents submitting to p4 from git due to missing hunks. Optional shrinking of RCS keywords in git-p4. New option git-p4.kwstrip set to true or false controls the behavior..

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2216cac..ac8b7f7 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -16,6 +16,9 @@ from sets import Set;

 verbose = False

+# Handling of RCS keyowrds. To ensure backward compatibility, the default
+# is to strip keywords. Default behavior is controlled here
+kwstrip = True

 def p4_build_cmd(cmd):
     """Build a suitable p4 command line.
@@ -975,7 +978,9 @@ class P4Sync(Command):
                 sys.stderr.write("p4 print fails with: %s\n" % repr(stat))
                 continue

-            if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
+            if not kwstrip:
+               pass
+            elif stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
                 text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
             elif stat['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'binary+k'):
                 text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$',r'$\1$', text)
@@ -1850,6 +1855,16 @@ def main():
         (cmd, args) = parser.parse_args(sys.argv[2:], cmd);
     global verbose
     verbose = cmd.verbose
+
+    global kwstrip
+    kwval = gitConfig("git-p4.kwstrip")
+    if len(kwval) > 0:
+       kwval = kwval.lower();
+        if kwval == "false":
+            kwstrip = False
+        elif kwval == "true":
+            kwstrip = True
+
     if cmd.needsGit:
         if cmd.gitdir == None:
             cmd.gitdir = os..path.abspath(".git")


-dhruva


----- Original Message ----
> From: Simon Hausmann <simon@lst.de>
> > +       kwval = kwval.lower();
> > +        if "false" == kwval:
> > +            kwstrip = False
> > +        elif "true" == kwval:
> > +            kwstrip = True
> 
> I have another style nitpick, sorry :). Please use "kwval == "false" instead 
> of the other way around.

There was a reason for keeping the constant as lvalue to avoid typos like '=' instead from '==' from 'C' school, I realize that python throws an error when such things happen. Must say that programming languages are becoming smarter and taking away the charm of programming...
 
> Otherwise your patch looks good to me, I think it's a very good option to add. 
> Please resend with commit message so that Junio can include it.


      Add more friends to your messenger and enjoy! Go to http://in.messenger.yahoo.com/invite/

Re: PATCH: git-p4 optional handling of RCS keywords

[Tor Arvid Lund]

On Thu, Sep 11, 2008 at 6:06 AM, dhruva <dhruva@ymail.com> wrote:
> Hello,
>
>
> Commit message: Modifying RCS keywords prevents submitting to p4 from git due to missing hunks. Optional shrinking of RCS keywords in git-p4. New option git-p4.kwstrip set to true or false controls the behavior..
<snip away the diff>

Me guesses that Junio (who gets loads of patches) would be happy if you do:

1) Squash these changes down to one single commit
2) Run git format-patch -1
3) Send it to the list either with git send-email, or otherwise send
the 0001-<your-commit-summary>.patch to the list directly if your
mailer supports it.

The reason is simply that the maintainer can simply use git am
<your-raw-mail-message-file> to commit it, instead of having to
manually sift through the emails.

Otherwise, this is nice work. Thanks.

-Tor Arvid-

Re: PATCH: git-p4 optional handling of RCS keywords

[dhruva]

Hi,


 The patch I have sent is 1 single patch.


----- Original Message ----
> From: Tor Arvid Lund <torarvid@gmail.com>
> To: dhruva <dhruva@ymail.com>
> Me guesses that Junio (who gets loads of patches) would be happy if you do:
> 
> 1) Squash these changes down to one single commit
> 2) Run git format-patch -1
> 3) Send it to the list either with git send-email, or otherwise send
> the 0001-.patch to the list directly if your
> mailer supports it.

Let me start learning that now. I have added my own extensions on top of what I have submitted, I need to figure out a way to do all these stuff (I am still a newbie to git ;)

-dhruva



      Get an email ID as yourname@ymail.com or yourname@rocketmail.com. Click here http://in.promos.yahoo.com/address

Re: PATCH: git-p4 optional handling of RCS keywords

[dhruva]

>From aa822b35925b893de049b6cc4d1531dc476ade56 Mon Sep 17 00:00:00 2001
From: Dhruva Krishnamurthy <dhruva@siml6.eng.btc.netapp.in>
Date: Thu, 11 Sep 2008 14:36:32 +0530
Subject: [PATCH] Modifying RCS keywords prevents submitting to p4 from git due to missing hunks. Optional shrinking of RCS keywords in git-p4. New option git-p4.kwstrip set to true or false controls the behavior


Signed-off-by: Dhruva Krishnamurthy <dhruva@ymail.com>
---
 contrib/fast-import/git-p4 |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2216cac..ac8b7f7 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -16,6 +16,9 @@ from sets import Set;

 verbose = False

+# Handling of RCS keyowrds. To ensure backward compatibility, the default
+# is to strip keywords. Default behavior is controlled here
+kwstrip = True

 def p4_build_cmd(cmd):
     """Build a suitable p4 command line.
@@ -975,7 +978,9 @@ class P4Sync(Command):
                 sys.stderr.write("p4 print fails with: %s\n" % repr(stat))
                 continue

-            if stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
+            if not kwstrip:
+               pass
+            elif stat['type'] in ('text+ko', 'unicode+ko', 'binary+ko'):
                 text = re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text)
             elif stat['type'] in ('text+k', 'ktext', 'kxtext', 'unicode+k', 'binary+k'):
                 text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$',r'$\1$', text)
@@ -1850,6 +1855,16 @@ def main():
         (cmd, args) = parser.parse_args(sys.argv[2:], cmd);
     global verbose
     verbose = cmd.verbose
+
+    global kwstrip
+    kwval = gitConfig("git-p4.kwstrip")
+    if len(kwval) > 0:
+       kwval = kwval.lower();
+        if kwval == "false":
+            kwstrip = False
+        elif kwval == "true":
+            kwstrip = True
+
     if cmd.needsGit:
         if cmd.gitdir == None:
             cmd.gitdir = os.path.abspath(".git")
--
1.6.0.1.442.g17b2f

-dhruva



      Get an email ID as yourname@ymail.com or yourname@rocketmail.com. Click here http://in.promos.yahoo.com/address