Approvals: 0/1
[01:27:47] <paveldrankov> hello guys
[01:27:59] <paveldrankov> how are you doing?
[11:34:42] *** ChanServ sets mode: +o temporalfox
[15:53:06] <yunyul> Trying to fix something in the template engines, but MVEL test is like wtf
[15:53:07] <yunyul> https://i.imgur.com/6YpnfP2.png
[15:53:31] <yunyul> could it be a windows newline issue?
[15:54:47] <yunyul> Yep, it was
[15:54:49] <yunyul> lol
[16:02:24] <yunyul> Read scrollback, it's for literally everything
[16:02:26] <yunyul> so for the MVEL tests
[16:02:37] <yunyul> I narrowed it down to MVELTemplateTestMVELTemplateTestMVELTemplateTest and testTemplateHandlerWithInclude failing
[16:07:38] <yunyul> and both are filesystem template tests
[16:07:46] <yunyul> AlexLehm_, it's not git related
[16:07:55] <yunyul> the MVEL engine renders windows line endings if it's running on windows
[16:08:08] <yunyul> the templates are actually unix-style
[16:11:14] <yunyul> or is it the templates?
[16:11:18] <yunyul> ran dos2unix and the tests are passing
[16:11:18] <yunyul> hmmm
[16:17:53] <yunyul> OK, it's just MVEL
[17:16:24] <yunyul> Hmm
[17:16:34] <yunyul> This presents a bigger problem since we can't rely on System.lineSeparator for the tests
[17:16:38] <yunyul> because people might have different git configs
[17:29:12] <AlexLehm_> yunyul: it might be possible to write a comparison check that ignores line endings or forces the eol to \n regardless of the line separator setting
[17:29:43] <AlexLehm_> I think I implemented that somewhere but I haven't found it yet
[17:30:57] <AlexLehm> something like if string.contains(lineSeparator) string=string.replace(lineSeparator, “\n”)
[17:34:26] <yunyul> that's what i PRed but I realized that it wouldn't work for some situations
[17:35:08] <yunyul> going to try to see if i can just normalize it in readFileToString
[17:35:18] <yunyul> '\n' in used in the rest of the project so might as well fix it at the source
[17:46:17] <yunyul> hmm that won't work for Freemarker
[17:46:23] <yunyul> since vertx doesn't handle file reading for freemarker
[17:52:08] <yunyul> Yeah we shouldn't actively break template engine behavior
[17:54:34] <yunyul> oh actually freemarker has some code repetition lol
[18:09:46] <yunyul> Alright, think I fixed it
[18:10:06] <yunyul> Also found out that Freemarker template loader could use a bit of improvement
[18:20:00] <yunyul> AlexLehm, would you mind looking at this PR
[18:20:00] <yunyul> https://github.com/vert-x3/vertx-web/pull/570
[20:35:51] <AlexLehm> yunyul: looks ok, i added a check to not do the conversion when not in windows
[20:35:51] <AlexLehm> https://github.com/alexlehm/vertx-web/tree/fix-template-tests-on-windows
[20:36:55] <AlexLehm> the pebble test is failing for me for some reason
[21:06:14] <yunyul> AlexLehm, weird, all tests pass for me
[21:08:34] <yunyul> oh i see
[21:08:36] <yunyul> pebble is broken too
[21:16:55] <yunyul> that said we are completely ignoring charsets
[21:17:00] <yunyul> hmm
[21:17:55] <yunyul> oh ez fix
[21:24:31] <yunyul> hmm charsets are the issue
[21:46:53] <yunyul> AlexLehm, can you try out the latest commits with pebble
[22:03:08] <yunyul> encoding is the problem
[22:10:58] <yunyul> Did a commit to fix the encoding
[22:40:26] <AlexLehm> yunyul: tests work for me now. i have sent you a pr since i think the operation should only be done when necessary