You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/12/11 12:38:52 UTC
[GitHub] [logging-log4cxx] coldtobi opened a new pull request #81: Don't convert CRLF, git
coldtobi opened a new pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81
By default (at least on Linux) git does convert CRLF automatically, also in the
above mentioned file. However, that CRLF is a feature in that file....
Jira: LOGCXX-540
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991706119
> There's one line in the properties file per test-case. So it does test \r\n on *nix.[...]
Which means currently the properties parser is tested with mixed line endings as well, which wouldn't be the case anymore if two different files with their unique line endings would be used?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991653650
Seems that some editors like my Sublime Text 3 don't even show something like mixed line endings, but it claims the file to be `Unix` only. The line of interest is the following:
```
propertiestestcase.crlf1=continued\
value
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi edited a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi edited a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-997165067
> I've made a new PR with the changes, if you are able to review that would be helpful: #85
#85 works for me; I guess then lets close this PR, as #85 replaces it. Kudos to Robert for your work!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi closed pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi closed pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi edited a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi edited a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991684540
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi edited a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi edited a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-997165067
> I've made a new PR with the changes, if you are able to review that would be helpful: #85
#85 works for me; I guess then lets close this PR, as #85 replaces it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening removed a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening removed a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991674503
If it doesn't break anything else, agreed, your approach would be better. Especially if that use-case is already covered in `.gitattributes`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi closed pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi closed pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-997136517
I've made a new PR with the changes, if you are able to review that would be helpful: https://github.com/apache/logging-log4cxx/pull/85
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-997165067
> I've made a new PR with the changes, if you are able to review that would be helpful: #85
#85 works for me; I guess then lets close this PR, as #85 succeeds it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi edited a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi edited a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991684540
I'm not sure about what changes requested:
- The suggestion from @ams-tschoening for formatting the gitattributes file
- rm5248's comment ?
I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix), as the properties are named accordingly (the extra crlf in the properties name)
(Sorry, hit accidentially the close button)
FWIIW, vim shows the CRLF as ^M:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi closed pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi closed pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991664058
I was actually just about to go and fix this by creating a new file that was in Windows(CRLF) line endings, since that particular property is only used in [one test.](https://github.com/apache/logging-log4cxx/blob/d08a933041c87c7a68908334df5b0cceadd3a7f5/src/test/cpp/helpers/propertiestestcase.cpp#L158) Is there some reason we want to have a file with mixed line endings? Keeping it separate seems like the best idea to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991709500
> > There's one line in the properties file per test-case. So it does test \r\n on *nix.[...]
>
> Which means currently the properties parser is tested with mixed line endings as well, which wouldn't be the case anymore if two different files with their unique line endings would be used?
That looks like more of an unintentional side-effect of the test, rather than something that is checked explicitly. I think this needs to be split into the following parts, so that we test everything explicitly:
- The current test(minus the current crlf property)
- `\n` on all platforms(propertiestsetcase-lf.properties)
- `\r\n` on all platforms(propertiestestcase-crlf.properties)
- Mixed line endings(propertiestestcase-mixed.properties)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991684540
I'm not sure about what changes requested:
- The suggestion from @ams-tschoening for formatting the gitattributes file
- rm5248's comment
I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991653650
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991664058
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening removed a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening removed a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991705032
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991703645
> I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix), as the properties are named accordingly (the extra crlf in the properties name)
There's one line in the properties file per test-case. So it does test `\r\n` on *nix. There's one line in the properties file per test-case defined.
My proposal is to simply create a new file(propertiestestcase-crlf.properties or something like that) that only has one line in it(apart from the standard Apache header):
```
propertiestestcase.crlf1=continued\^M
value
```
That way, we can set the .gitattributes file to set propertiestestcase-crlf.properties to be `eol=crlf`:
```
src/main/resources/propertiestestcase-crlf.properties eol=crlf
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991674503
If it doesn't break anything else, agreed, your approach would be better. Especially if that use-case is already covered in `.gitattributes`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi edited a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi edited a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991684540
I'm not sure about what changes requested:
- The suggestion from @ams-tschoening for formatting the gitattributes file
- rm5248's comment
I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix)
(Sorry, hit accidentially the close button)
FWIIW, vim shows the CRLF as ^M:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi edited a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi edited a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991684540
I'm not sure about what changes requested:
- The suggestion from @ams-tschoening for formatting the gitattributes file
- rm5248's comment ?
I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix)
(Sorry, hit accidentially the close button)
FWIIW, vim shows the CRLF as ^M:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi edited a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi edited a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991684540
I'm not sure about what changes requested:
- The suggestion from @ams-tschoening for formatting the gitattributes file
- rm5248's comment
I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix)
(Sorry, hit the wrong button)
FWIIW, vim shows the CRLF as ^M:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
coldtobi commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991684540
I'm not sure about what changes requested:
- The suggestion from @ams-tschoening for formatting the gitattributes file
- rm5248's comment
I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening removed a comment on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening removed a comment on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991705032
> I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix), as the properties
> are named accordingly (the extra crlf in the properties name)
Having a second thought, that's how I understand it as well now:
```
propertiestestcase.tab7=continued\
value
propertiestestcase.crlf1=continued\
value
```
The first of those properties is not only TAB-focused, but LF as well, otherwise the continued values for those both wouldn't make too much sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on pull request #81: Don't convert CRLF, git
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on pull request #81:
URL: https://github.com/apache/logging-log4cxx/pull/81#issuecomment-991705032
> I thought that the purpose of the testcase is to test both possiblities of line continuations (on Win and *nix), as the properties
> are named accordingly (the extra crlf in the properties name)
Having a second thought, that's how I understand it as well now:
```
propertiestestcase.tab7=continued\
value
propertiestestcase.crlf1=continued\
value
```
The first of those properties is not only TAB-focused, but LF as well, otherwise the continued values for those both wouldn't make too much sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org