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

fixed: last file terminating newline #3

Conversation

ncaq
Copy link

@ncaq ncaq commented Apr 24, 2018

From POSIX.
Text file terminating newline.
project-template had bug to no newline last file of hsfiles.
Because, files split by two newline.
But, last file have one newline, Do not two newline.
This commit compatibility omit existing hsfiles.
hsfiles last file do not need to have two newline, and get POSIX text file by this commit.

From [POSIX](http://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap03.html#tag_03_392).
Text file terminating newline.
project-template had bug to no newline last file of hsfiles.
Because, files split by two newline.
But, last file have one newline, Do not two newline.
This commit compatibility omit existing hsfiles.
hsfiles last file do not need to have two newline, and get POSIX text file by this commit.
@snoyberg
Copy link
Member

I'm sorry, but I'm having trouble understanding the bug report. The current version of the format allows you to create an hsfiles either with or without the trailing endline. I don't see any violation of a POSIX spec in this, the author of a file has control over how this is rendered.

@ncaq
Copy link
Author

ncaq commented Apr 26, 2018

For example, case new-template.

% stack new foobar

And you open the .gitignore

% emacs foobar/.gitignore

This file don't terminating newline.
This file is not POSIX file.

Because, other file split of two newline in hsfiles.

For example,

{-# START_FILE Setup.hs #-}
import Distribution.Simple
main = defaultMain

{-# START_FILE test/Spec.hs #-}
main :: IO ()
main = putStrLn "Test suite not yet implemented"

However, last hsfile file is not have two newline.

I think to fix to commercialhaskell/stack-templates: Project templates for stack new.
However, this is trouble.
And, some text editor(Emacs whitespace-mode…) wrong to two newline at file last.

Should I fix to stack-templates or what?

@snoyberg
Copy link
Member

I haven't checked this myself yet, but doesn't this patch force a trailing newline on binary content files as well? Files like images should definitely not have such a character appended.

I lean towards modifying the templates themselves instead of the library.

@ncaq
Copy link
Author

ncaq commented May 1, 2018

Files like images should definitely not have such a character appended.

I think to base64 ignore newline.

> B64.decodeLenient "abc"
"i\183"
> B64.decodeLenient "abc\n"
"i\183"
> B64.decodeLenient "abc\n\n\n"
"i\183"

The base64 spec allow newline in body.
Because, email limit width character.


I do not care about how to resolve the POSIX file problem.
You can fix it by modifying stack-templates.
However, since many text editors eliminate useless line breaks, it is difficult for authors to be careful one by one.

@snoyberg
Copy link
Member

snoyberg commented May 4, 2018

This does in fact break idempotence for textual files:

{-# LANGUAGE OverloadedStrings #-}
import Text.ProjectTemplate
import Conduit
import qualified Data.ByteString as B
import Control.Monad.Writer

main :: IO ()
main = do
  let orig = "hello"
  bs <- runConduit $ yield ("text", return orig) .| createTemplate .| foldC
  m <- execWriterT $ runConduit $ yield bs .| unpackTemplate receiveMem id
  print m

Results in:

fromList [("text","hello\n")]

Note that an extra newline has been appended. Ultimately, this comes down to the fact that the change you're proposing provides no way to handle the case where a textual file doesn't have a trailing newline. You may claim that this shouldn't be allowed, but nonetheless this breaks all existing templates that use text files and correctly encode the trailing newline.

@snoyberg
Copy link
Member

snoyberg commented May 4, 2018

Actually, I just realized that the original test suite caught this bug, and your changes simply hide the bug: https://github.com/fpco/haskell-ide/pull/3/files#diff-891fa61a464b745f131591522cdd6dfbR30.

@ncaq
Copy link
Author

ncaq commented May 8, 2018

OK, I respect compatibility.
This pull request close.
We should fix to stack-templates.

@ncaq ncaq closed this May 8, 2018
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.

None yet

2 participants