Reference.from_path now only creates references, not symbolic refs. SymbolicReference.from_path creates only symbolic refs. This change was not detected by a test, hence there is room for improvement on the testing field

Byron · Byron · commit 9b426893ce33 · 2009-11-03T18:04:40.000+01:00
diff --git a/TODO b/TODO
@@ -85,15 +85,11 @@ Refs
   handles symbolic refs as well as normal refs, updating the reflog if required.
 * I have read that refs can be symbolic refs as well which would imply the need
   to possibly dereference them. This makes sense as they originally where possibly 
-  a symbolic link
-* Check whether we are the active reference HEAD.reference == this_ref
-  - NO: The reference dosnt need to know - in fact it does not know about the 
-  main HEAD, so it may not use it. This is to be done in client code only. 
-  Remove me
-* Reference.from_path may return a symbolic reference although it is not related 
-  to the reference type. Split that up into two from_path on each of the types, 
-  and provide a general method outside of the type  that tries both.
-* Making the reflog available might be useful actually.
+  a symbolic link. This would mean References could be derived from SymbolicReference
+  officially, but it would still be bad as not all References are symbolic ones.
+* Making the reflog available as command might be useful actually. This way historical 
+  references/commits can be returned. Git internally manages this if refs are specified
+  with HEAD@{0} for instance
 
 Remote
 ------
diff --git a/lib/git/refs.py b/lib/git/refs.py
@@ -151,18 +151,12 @@ def iter_items(cls, repo, common_path = None, **kwargs):
 	def from_path(cls, repo, path):
 		"""
 		Return
-			Instance of type Reference, Head, Tag, SymbolicReference or HEAD
+			Instance of type Reference, Head, or Tag
 			depending on the given path
 		"""
 		if not path:
 			raise ValueError("Cannot create Reference from %r" % path)
 		
-		if path == 'HEAD':
-			return HEAD(repo, path)
-		
-		if '/' not in path:
-			return SymbolicReference(repo, path)
-			
 		for ref_type in (Head, RemoteReference, TagReference, Reference):
 			try:
 				return ref_type(repo, path)
@@ -218,7 +212,10 @@ class SymbolicReference(object):
 	
 	def __init__(self, repo, name):
 		if '/' in name:
+			# NOTE: Actually they can be looking like ordinary refs. Theoretically we handle this
+			# case incorrectly
 			raise ValueError("SymbolicReferences are not located within a directory, got %s" % name)
+		# END error handling 
 		self.repo = repo
 		self.name = name
 		
@@ -348,6 +345,23 @@ def is_detached(self):
 		except TypeError:
 			return True
 	
+	@classmethod
+	def from_path(cls, repo, path):
+		"""
+		Return
+			Instance of SymbolicReference or HEAD
+			depending on the given path
+		"""
+		if not path:
+			raise ValueError("Cannot create Symbolic Reference from %r" % path)
+		
+		if path == 'HEAD':
+			return HEAD(repo, path)
+		
+		if '/' not in path:
+			return SymbolicReference(repo, path)
+			
+		raise ValueError("Could not find symbolic reference type suitable to handle path %r" % path)
 	
 class HEAD(SymbolicReference):
 	"""
