Use config Newline Setting In System Attribute Evaluation (#154)

[hoadlck]

Problems seen with inconsistent newlines in the Table Of Contents HTML pulled in from the configuration file. While the rest of the contents in the resulting HTML file honored the newline style configured by the user, this specific snippet of HTML always used "\r\n".

This root of this problem existed for some time, but in earlier versions, the newline style was always "\n". The symptoms changed as a side effect of the issue "Extra line padding in source and literal blocks" (#139). In this issue, the newline style changed to "\r\n", and was noticed by users.

Change the System Attribute Evaluation function to use the newline setting from the global "config" instead of using the default newline.

Closes #154

[MasterOdin]

Can we also change these two lines to use config.newline while we're at it:
https://github.com/asciidoc/asciidoc-py3/blob/89ac0f48affb054226467b58f5af524ae0760d62/asciidoc.py#L3637-L3638
https://github.com/asciidoc/asciidoc-py3/blob/89ac0f48affb054226467b58f5af524ae0760d62/asciidoc.py#L5656

Could you add a test case of a file that specifies newline as a configuration option in the test suite? (Feel free to say no, and I can handle this).

I'll take a look at the test suite today to get that working.

[hoadlck]

This is for #154.

I noticed two other areas where DEFAULT_NEWLINE was used, and I did not change them. Both of these were in the context of reading csv files. If they are an issue (which I would need to research more to see if they are), then I don't know if it would be preferred to put it in another PR, or extend this one.

[MasterOdin]

Fair enough, I did not look very deeply into it, I'll open a separate issue to track that, and we can leave this PR to just focus on the use-case of #154.

[hoadlck]

Looks like the tests are failing because of some commands that GitHub deprecated. They no longer allow the app-path commands. The error referenced this documenation.

Here is the error:

Error: Unable to process command '::add-path::/home/runner/venv-3.10/bin' successfully.
Error: The `add-path` command is disabled. Please upgrade to using Environment Files or opt into unsecure command execution by setting the `ACTIONS_ALLOW_UNSECURE_COMMANDS` environment variable to `true`. For more information see: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/

The tests did run fine when I ran them locally.

pi@maa:~/asciidoc-py3/tests $ time python3 testasciidoc.py run
<snip>
PASSED: html4: data/lang-en-last-updated-is-revdate-test-html4.html
PASSED: html5: data/lang-en-last-updated-is-revdate-test-html5.html

TOTAL PASSED:  246
TOTAL SKIPPED: 15

real    4m2.401s
user    3m58.967s
sys     0m3.427s
pi@maa:~/asciidoc-py3/tests $ uname -a
Linux maa 5.4.72-v7+ #1356 SMP Thu Oct 22 13:56:54 BST 2020 armv7l GNU/Linux
pi@maa:~/asciidoc-py3/tests $ python3 --version
Python 3.7.3

[MasterOdin]

Yup, easy enough fix in #156 and now on master. Would you mind merging master into your branch?

[MasterOdin]

Thanks. Last bit was just confirming if you think you could add a test case for this?

[hoadlck]

Sure, I can add a test case. It will be tonight (or maybe tomorrow) before I will have time to look at it though. But, I have wanted to understand how the tests worked for some time.

If you want to get the release out sooner than that, it won't hurt my feelings if you add the test case. :-)

[MasterOdin]

👍

I'm fine waiting a few days for you to write the test if it means having more collaborators know the repository more fully.

[hoadlck]

I pushed an update with the tests for newlines. I added a new test input file that has 3 sections: each section is in a different newline style (DOS/UNIX/MAC). I enabled the TOC so that the original issue would be visible if it ever came back.

Let me know if there are any changes you think would be a good idea.

[hoadlck]

I looked into the uses of DEFAULT_NEWLINE where it is used in csv.reader(). The usage is in the context of reading a .csv file to determine the contents of a table. Since the individual cells are extracted by the csv reader, the new lines do not really matter.

It looks like the caller of the parse_csv() method has already broken the csv lines into a list, so the DEFAULT_NEWLINE.join(text) is just a way to put the text into a format that the normal csv reader code can parse it. I tried having the csv with different line endings, but it did not make any difference in the output.

I even tried making a cell have multiple lines in the csv. The output HTML did honor the specified newline (not the newline that was embedded in the csv). Of course, the rendered text for the multi-line cell all appeared to be on one line. Which seems like the correct functionality.

I don't see any tests that use a csv file as input for a table. Do you want me to include one in this PR, or reserve that for another?

[hoadlck]

I don't see any tests that use a csv file as input for a table. Do you want me to include one in this PR, or reserve that for another?

Never mind...I see that the tables test is using a csv from the website area. So, I think this PR is ready as-is.

[MasterOdin]

Thanks for this! The tests look good and the generated html files have the expected line endings. Did a manual test on #154 with provided guide.txt and asciidoc.conf with just the specified attributes and that looks good as well.