Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default file.multiline.mode to halt_before #1897

Open
binarylogic opened this issue Feb 22, 2020 · 2 comments
Open

Default file.multiline.mode to halt_before #1897

binarylogic opened this issue Feb 22, 2020 · 2 comments
Labels
meta: idea Anything in the idea phase. Needs further discussion and consensus before work can begin. needs: approval Needs review & approval before work can begin. source: file Anything `file` source related type: enhancement A value-adding code change that enhances its existing functionality.

Comments

@binarylogic
Copy link
Contributor

In my opinion, what's happening here should be the default. This preserves the old multi-line behavior where only a start_pattern is required. More sophisticated matching can then use mode and condition_pattern.

@MOZGIII what do you think?

@binarylogic binarylogic added source: file Anything `file` source related type: enhancement A value-adding code change that enhances its existing functionality. labels Feb 22, 2020
@MOZGIII
Copy link
Contributor

MOZGIII commented Feb 23, 2020

I can understand having that default for compatibility with our old message_start_indicator - but we already have the backward compatibility for that (the code you linked is responsible for that).
On its own, I don't think it's really a good default. What it does is it takes away the question users would have to ask themselves - what's the right way to do the parsing.
What I'm trying to say is I'm afraid that by providing a default we'll be suggesting that one of the modes is "better" than the others.

So, yeah. I don't know. On one side - convention over configuration is nice, on the other - it's not really a good default, and we might also want to force users to think about how this is configured. After all, this feature is opt-in, and providing this kind of default doesn't allow us to enable it by default for people who didn't care to configure it... I'm a bit hesitant to add it, as I don't really see a good purpose. But if you still think we need it - let's do it.

@MOZGIII
Copy link
Contributor

MOZGIII commented Feb 23, 2020

What we could do though is to improve the user experience with the configuration parameters! As proposed here at #1431, instead of what we have currently, we can expose the same four modes by making users specify different fields:

  • continue through

    • start_pattern
    • continue_through_pattern
  • continue past

    • start_pattern
    • continue_past_pattern
  • halt before

    • start_pattern
    • halt_before_pattern (optional, defaults to start_pattern value)
  • halt with

    • start_pattern
    • stop_patternit's

This solution kind of includes the default but in a way that it explicitly specifies the mode when the user doesn't set anything but start_pattern.

I personally like it less than the more explicit and KISS approach that we have currently - mostly cause I don't like the value it provides for the maintenance efforts for the code that would implement this fancy mapping. It also doesn't help with multi-line parsing being an opt-in feature. But at least it's fewer lines of configuration for the end-users, which is an improvement.

@binarylogic binarylogic added meta: idea Anything in the idea phase. Needs further discussion and consensus before work can begin. needs: approval Needs review & approval before work can begin. labels Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: idea Anything in the idea phase. Needs further discussion and consensus before work can begin. needs: approval Needs review & approval before work can begin. source: file Anything `file` source related type: enhancement A value-adding code change that enhances its existing functionality.
Projects
None yet
Development

No branches or pull requests

2 participants