This version (2017/05/27 13:44) is a draft.
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