diff mbox

[kteam-tools] ktl: Git.current_branch() optimization

Message ID 1369433208-17373-1-git-send-email-kamal@canonical.com
State New
Headers show

Commit Message

Kamal Mostafa May 24, 2013, 10:06 p.m. UTC
Query 'git symbolic-ref HEAD' instead of iterating the 'git branch' list.

Also changed behavior if current_branch() is called in a detached HEAD state:
  before: returned the string "(no branch)"
  now: throws an exception ktl.git.GetError GitError("no current branch")

No existing caller depends on the old "(no branch)" behavior.

Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 ktl/git.py | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

Comments

Brad Figg May 30, 2013, 7:15 p.m. UTC | #1
On 05/24/2013 03:06 PM, Kamal Mostafa wrote:
> Query 'git symbolic-ref HEAD' instead of iterating the 'git branch' list.
> 
> Also changed behavior if current_branch() is called in a detached HEAD state:
>   before: returned the string "(no branch)"
>   now: throws an exception ktl.git.GetError GitError("no current branch")
> 
> No existing caller depends on the old "(no branch)" behavior.
> 
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  ktl/git.py | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/ktl/git.py b/ktl/git.py
> index 9483ad4..7b18275 100644
> --- a/ktl/git.py
> +++ b/ktl/git.py
> @@ -94,17 +94,10 @@ class Git:
>      #
>      @classmethod
>      def current_branch(cls):
> -        retval = ""
> -        status, result = run_command("git branch", cls.debug)
> -        if status == 0:
> -            for line in result:
> -                if line != '' and line[0] == '*':
> -                    retval = line[1:].strip()
> -                    break
> -        else:
> -            raise GitError(result)
> -
> -        return retval
> +        status, result = run_command("git symbolic-ref --short HEAD", cls.debug)
> +        if status != 0:
> +            raise GitError("no current branch")
> +        return result[0]
>  
>      # show
>      #
> 

This doesn't work correctly on Precise, probably different git version. Feel
free to try again.
Kamal Mostafa May 30, 2013, 7:55 p.m. UTC | #2
On Thu, 2013-05-30 at 12:15 -0700, Brad Figg wrote:
> On 05/24/2013 03:06 PM, Kamal Mostafa wrote:
> > Query 'git symbolic-ref HEAD' instead of iterating the 'git branch' list.
> > 

> This doesn't work correctly on Precise, probably different git version. Feel
> free to try again.
> 

Oops!  The forthcoming PATCHv2 should work (tested as far back as
lucid).

 -Kamal
diff mbox

Patch

diff --git a/ktl/git.py b/ktl/git.py
index 9483ad4..7b18275 100644
--- a/ktl/git.py
+++ b/ktl/git.py
@@ -94,17 +94,10 @@  class Git:
     #
     @classmethod
     def current_branch(cls):
-        retval = ""
-        status, result = run_command("git branch", cls.debug)
-        if status == 0:
-            for line in result:
-                if line != '' and line[0] == '*':
-                    retval = line[1:].strip()
-                    break
-        else:
-            raise GitError(result)
-
-        return retval
+        status, result = run_command("git symbolic-ref --short HEAD", cls.debug)
+        if status != 0:
+            raise GitError("no current branch")
+        return result[0]
 
     # show
     #