Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing #15 #26

Merged
merged 2 commits into from
May 15, 2012
Merged

Fix parsing #15 #26

merged 2 commits into from
May 15, 2012

Conversation

dedan
Copy link
Contributor

@dedan dedan commented May 14, 2012

this should fix the problem described in #15.

I did not change much but it should already do the trick.

You think this is enough? please comment..

@maebert
Copy link
Contributor

maebert commented May 15, 2012

Looks good. Not sure what the best practise regarding line separators is, though - should we use the system's default, or check what newline sequence the journal file uses and use that one for writing as well (ie. if the journal was created on Windows, use CR+LF even os.linesep is LF on *nix?

maebert pushed a commit that referenced this pull request May 15, 2012
Fix parsing of newlines, closes #15
@maebert maebert merged commit 4329979 into jrnl-org:master May 15, 2012
@dedan
Copy link
Contributor Author

dedan commented May 16, 2012

first I thought system separator should be fine. But you are right,
then we would run into problems when
someone uses the same file via dropbox on a win and a *nix machine. But who uses
commandline applications on on win anyway?

I just saw that you already merged in my latest changes, I think this
was a bit premature
because I had some strange behavior yesterday. Will look into it as
soon as I'll find some time.
Maybe you want to revert the merge until then?

And the last thing: should we maybe start adding tests?

On Tue, May 15, 2012 at 11:57 AM, Manuel Ebert
reply@reply.github.com
wrote:

Looks good. Not sure what the best practise regarding line separators is, though - should we use the system's default, or check what newline sequence the journal file uses and use that one for writing as well (ie. if the journal was created on Windows, use CR+LF even os.linesep is LF on *nix?


Reply to this email directly or view it on GitHub:
https://github.com/maebert/jrnl/pull/26#issuecomment-5712345

@dedan
Copy link
Contributor Author

dedan commented May 16, 2012

false alarm, strange behavior is not reproducable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants