search

logstash-plugins / logstash-codec-line

Apache License 2.0
4 stars 13 forks source link

Should default to \r\n on windows #10

Open andrewvc opened 8 years ago

andrewvc commented 8 years ago

When running on a windows box this should default to \r\n as a delimiter. This works pretty poorly ith the stdin input without this.

pickypg commented 8 years ago

I wonder if we can specify this so that it's the equivalent of \r?\n all of the time. I can't think of anyone that actually wants the \r in there, whether they're using a file or stdin.

wiibaa commented 8 years ago

One first improvment would be to use java System.lineSeparator() to set the default, feasible?

jordansissel commented 8 years ago

Maybe we make it a setting, maybe called "line_terminator" and have valid settings of "windows", "mac", "unix", or "network" ? (windows and network are identical, \r\n; mac is \r, unix is \n)

pickypg commented 8 years ago

But Mac doesn't do \r anymore. It does \n now. But, it is a good point.

As long as it's settable, then it really doesn't make a huge difference to me, but I think the default should really be "either of (\n|\r\n)" or "any of (\r|\n|\r\n)". There is an undocumented delimiter field that serves this purpose, but I think it requires it to be the exact delimiter rather than the match:

https://github.com/logstash-plugins/logstash-codec-line/blob/ceee8659e7bac14911a39d9662494632a4e25504/lib/logstash/codecs/line.rb#L31

andrewvc commented 8 years ago

@jordansissel this plugin already has a 'delimiter' setting. I'm really curious if it works correctly with escaped characters in the logstash config however.

andrewvc commented 8 years ago

Also, the only reason delimiter is undocumented is that our doc generator hasn't run recently on this.

andrewvc commented 8 years ago

I think @wiibaa makes a great point, but I agree that we should look for either \r\n or \n. No one wants the \r.

jordansissel commented 8 years ago

Some Mac apps still use \r (FileMaker) even on OS X. :/

On Monday, June 20, 2016, Chris Earle notifications@github.com wrote:

But Mac doesn't do \r anymore. It does \n now. But, it is a good point.

As long as it's settable, then it really doesn't make a huge difference to me, but I think the default should really be "either of (\n|\r\n)" or "any of (\r|\n|\r\n)". There is an undocumented delimiter field that serves this purpose, but I think it requires it to be the exact delimiter rather than the match:

https://github.com/logstash-plugins/logstash-codec-line/blob/ceee8659e7bac14911a39d9662494632a4e25504/lib/logstash/codecs/line.rb#L31

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/logstash-plugins/logstash-codec-line/issues/10#issuecomment-227245170, or mute the thread https://github.com/notifications/unsubscribe/AAIC6o0ZGpvvhE9b57eM4DsrCCLWbzalks5qNurtgaJpZM4I6AdE .