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.

Sign up for GitHub

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

doc/flowint: fix terminology for unset variable - v1 #12159

Closed
wants to merge 1 commit into from
Closed

doc/flowint: fix terminology for unset variable - v1 #12159

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion doc/userguide/rules/flow-keywords.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,12 @@ Define a var (not required), or check that one is set or not set.
::

flowint: name, < +,-,=,>,<,>=,<=,==, != >, value;
flowint: name, (isset|isnotset);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My instinct is that we should keep such things consistent among the rule keywords. Of course, that comes at an expense of changing and breaking the existing rulesets that use this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rather change the code to support both historical notset but also generic isnotset ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds very good to me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. Should I create a ticket, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://redmine.openinfosecfoundation.org/issues/7426

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change the code? Why not just keep this to updating the docs to reflect reality?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above and in the ticket, to keep consistency among rule keywords syntax. Should we not aim for that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more referring to why close this one which corrects the documentation issue. The code change is a somewhat separate issue which is still good to have for consistency.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code change + doc will be a logical unit of some sort...

But let's not overthink it, I am ok either way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess... We can merge this, and then once the code change happens, another doc update will be due.

flowint: name, (isset|notset);

.. note::

It's important to observe that while similar keywords use ``isnotset``,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proper fix here is to add isnotset support, while keeping the notset for compatibility reasons

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this seems trivial enough. But not getting to it, should not prevent correct documentation :)

for ``flowint`` the term is ``notset``.

Compare or alter a var. Add, subtract, compare greater than or less
than, greater than or equal to, and less than or equal to are
Expand Down
Loading