Skip to content

Conversation

@bertwesarg
Copy link
Contributor

No description provided.

lfd=LockedFD(fpath)
fd=lfd.open(write=True, stream=True)
fd.write(write_value.encode('ascii'))
fd.write(write_value.encode('ascii')+'\n')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Python 3 compat, prefer to use b'\n' here, since this should be a byte, not a unicode string.

@Byron
Copy link
Member

@bertwesarg Thanks for your contribution. May I ask why this is needed ? When creating refs using plain git, I don't see newlines either.
In case this is a bug you are fixing, you could add a test that goes green with this patch.
Thanks for the clarification.

@bertwesarg
Copy link
ContributorAuthor

bertwesarg commented Jul 23, 2016

Hmm, while I still use an ancient git 2.1.4 it does add a newline:

$ git init . $ hexdump -C .git/HEAD 00000000 72 65 66 3a 20 72 65 66 73 2f 68 65 61 64 73 2f |ref: refs/heads/| 00000010 6d 61 73 74 65 72 0a |master.| 00000017 

And git master should still do this:

https://github.com/git/git/blob/master/refs/files-backend.c#L2740

Though I noticed this because the git prompt provided by git-prompt.sh did not worked anymore, because it uses the shell read builtin, which does not work if the file does not has a trailing newline:

https://github.com/git/git/blob/master/contrib/completion/git-prompt.sh#L433

$ __git_ps1 '%s\n' master $ truncate -s 22 .git/HEAD $ __git_ps1 '%s\n' $ 

@Byron
Copy link
Member

Thanks for the clarification and the thorough research, great to have this contribution merged now! I feel a bit silly that I was unable to 'see' the newline at the end of the symbolic ref with vim, but will certainly put hexdump on my map.

@ByronByron merged commit a4ad7ce into gitpython-developers:masterJul 23, 2016
@bertwesargbertwesarg deleted the patch-1 branch July 24, 2016 08:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@bertwesarg@Byron@nvie