Added Syntax Highlighting for nftables-firewall config file #3325

theredcmdcraft · 2024-05-30T21:07:17Z

Created nftables syntax highlighting

runtime/syntax/nftables.yaml

niten94 · 2024-06-09T14:53:48Z

Sorry, I do not know much about nftables syntax and syntax highlighting so I cannot review much. There is nothing else I can properly suggest but the header pattern is still at least not changed.

theredcmdcraft · 2024-06-09T14:56:23Z

Oh sorry, i didn`t see the "suggested change". I thought it had already been changed.

theredcmdcraft · 2024-06-23T21:08:21Z

Sorry, I do not know much about nftables syntax and syntax highlighting so I cannot review much. There is nothing else I can properly suggest but the header pattern is still at least not changed.

Can someone else check the code? Or can you merge it simply?

niten94

It may look weird if groups in IPv6 addresses with hex digits are not highlighted but integers can be highlighted using this rule:

    - constant.number: "\\b([1-9][0-9]*|0[0-7]*|0[Xx][0-9A-Fa-f]+)\\b"

There is a lot of expressions in nftables but the keywords suggested are usually used so it may be fine highlighting them only for now.

runtime/syntax/nftables.yaml

niten94 · 2024-06-24T13:33:50Z

Can someone else check the code? Or can you merge it simply?

I cannot merge pull requests but I tried reviewing again. I do not know much about syntax highlighting so I think it is better if another person reviews a bit. I think not adding all expressions and merging may be fine for now.

I looked at nft(8) manpage as a reference but I think reading it may be confusing when not looking at the wiki first.

theredcmdcraft · 2024-06-24T20:37:32Z

It may look weird if groups in IPv6 addresses with hex digits are not highlighted but integers can be highlighted using this rule:
    - constant.number: "\\b([1-9][0-9]*|0[0-7]*|0[Xx][0-9A-Fa-f]+)\\b"
There is a lot of expressions in nftables but the keywords suggested are usually used so it may be fine highlighting them only for now.

i think we must not check everything at the first time. there is also a tool called nft. with the command nft -c -f <path to file> you can check the nftables syntax before you restart the service. this will prompt you if there is an error in your config file.

runtime/syntax/nftables.yaml

niten94 · 2024-07-02T09:34:31Z

A git client will have to be used, but can you please squash the commits into one and amend on the latest commit after editing the file?

theredcmdcraft · 2024-07-12T18:49:59Z

A git client will have to be used, but can you please squash the commits into one and amend on the latest commit after editing the file?

Oh sorry i didn`t used a git client. I have accepted your suggestions. I saw your message little bit to late.

niten94 · 2024-07-12T19:19:49Z

I am sorry that I did not respond early weeks ago too. I think the file is fine now, but I do not know if the pull request will be merged if the commits are not squashed.

theredcmdcraft · 2024-07-12T19:22:15Z

I am sorry that I did not respond early weeks ago too. I think the file is fine now, but I do not know if the pull request will be merged if the commits are not squashed.

I don't know if i can Squash the commits.

niten94 · 2024-07-12T19:40:56Z

It is late here (3 AM) so I cannot assist much but something like these can be done:

sudo apt install git openssh-client
ssh-keygen -t ed25519 -f ~/.ssh/id_github

# open https://github.com/settings/keys in browser
# copy ~/.ssh/id_github.pub (not id_github) content and add key

exec ssh-agent bash
ssh-add ~/.ssh/id_github
git clone ssh://[email protected]/theredcmdcraft/micro.git
# reset to commit before first commit in this pull request
git reset --soft dfd3df5~1
# write commit message then save
# message can be just something like "Add nftables syntax highlighting"
git commit
git push -u origin master

There are still other pages about squashing you can read but this is what I would do.

theredcmdcraft · 2024-08-08T21:48:21Z

It is late here (3 AM) so I cannot assist much but something like these can be done:

sudo apt install git openssh-client
ssh-keygen -t ed25519 -f ~/.ssh/id_github

# open https://github.com/settings/keys in browser
# copy ~/.ssh/id_github.pub (not id_github) content and add key

exec ssh-agent bash
ssh-add ~/.ssh/id_github
git clone ssh://[email protected]/theredcmdcraft/micro.git
# reset to commit before first commit in this pull request
git reset --soft dfd3df5~1
# write commit message then save
# message can be just something like "Add nftables syntax highlighting"
git commit
git push -u origin master

There are still other pages about squashing you can read but this is what I would do.

I tried my best. Please have a look if this is correct. I hope you`re fine now.

niten94 · 2024-08-10T14:10:08Z

I am really sorry, it was not a good idea replying at 3 AM and I did not include --force in git push. I tested this with your branch on a private repository, but please try running these if possible:

exec ssh-agent bash
ssh-add ~/.ssh/id_github

# reset to first commit in pull request
git reset --soft dfd3df52
# save only
git commit --amend --date=now
git push --force -u origin master

Created nftables syntax highlighting

theredcmdcraft · 2024-09-08T13:15:41Z

I am really sorry, it was not a good idea replying at 3 AM and I did not include --force in git push. I tested this with your branch on a private repository, but please try running these if possible:
exec ssh-agent bash
ssh-add ~/.ssh/id_github

# reset to first commit in pull request
git reset --soft dfd3df52
# save only
git commit --amend --date=now
git push --force -u origin master

Sorry for the realy late reply. In last weeks there was a lot of stuff that i had to do. So hopefully now it sould be correctly squashed.

theredcmdcraft · 2024-10-05T11:49:02Z

Do you can merge it?

JoeKar

It tested it with some of our rules at work and it looks like it needs some more polish with a new PR.
@theredcmdcraft Do you like to support here? 😉

In general numbers like in...

micro/runtime/syntax/c.yaml

Lines 26 to 30 in 8c0e0fa

    
           - constant.number: "(\\b([1-9][0-9]*|0[0-7]*|0[Xx][0-9A-Fa-f]+|0[Bb][01]+)([Uu][Ll]?[Ll]?|[Ll][Ll]?[Uu]?)?\\b)" 
        
             # Decimal Floating Constants 
        
           - constant.number: "(\\b(([0-9]*[.][0-9]+|[0-9]+[.][0-9]*)([Ee][+-]?[0-9]+)?|[0-9]+[Ee][+-]?[0-9]+)[FfLl]?\\b)" 
        
             # Hexadecimal Floating Constants 
        
           - constant.number: "(\\b0[Xx]([0-9A-Za-z]*[.][0-9A-Za-z]+|[0-9A-Za-z]+[.][0-9A-Za-z]*)[Pp][+-]?[0-9]+[FfLl]?\\b)"

...would be nice too.

JoeKar · 2024-10-21T16:20:20Z

runtime/syntax/nftables.yaml

+    - preproc: "\\b(add|define|flush|include|delete)\\b"
+    - symbol: "[-=/:;,@]"
+    - symbol.operator: "[<>.&|^!]|\\b(and|ge|gt|le|lt|or|xor)\\b"
+    - constant.string: '([\"]{1})(.*)([\"]{1})'


The string handling is broken and I recommend to use a region for this purpose, similar to:

micro/runtime/syntax/c.yaml

Lines 40 to 42 in 8c0e0fa

- constant.string:

start: "'"

end: "'"

JoeKar · 2024-10-21T16:22:37Z

runtime/syntax/nftables.yaml

+    - statement: "\\b(accept|drop|goto|jump|log|masquerade|reject)\\b"
+    - preproc: "\\b(add|define|flush|include|delete)\\b"
+    - symbol: "[-=/:;,@]"
+    - symbol.operator: "[<>.&|^!]|\\b(and|ge|gt|le|lt|or|xor)\\b"


The != isn't properly handled as unequal-operator. I'd assume something similar to:

micro/runtime/syntax/c.yaml

Line 23 in 8c0e0fa

- symbol.operator: "[-+*/%=<>.:;,~&|^!?]|\\b(offsetof|sizeof)\\b"

Currently the = is tracked in line 14.

JoeKar · 2024-10-21T16:32:43Z

runtime/syntax/nftables.yaml

+
+rules:
+    - type: "\\b(chain|counter|map|rule|ruleset|set|table)\\b"
+    - type: "\\b(ether|icmp|icmpv6|icmpx|inet|ip|ip6|ipv4|ipv6|tcp|udp)\\b"


Could be reduced to i(cm)?p(x|(v?(4|6))?), but I'm unsure if it improves readability or regex compile time.

i think not that this can be reduced, because of then would be things like chain, counter, map etc and tcp udp not highlighed.

Argh, sure...forget to fill up: ether|inet|i(cm)?p(x|(v?(4|6))?)|tcp|udp 🤦‍♂️

I`ve added your suggestions, you can try if you want to. If i should build it for you, please answer me, then i will build it and create a little release for you.

But you can clone the Repo, and do the described manual build. (Don`t forget to install golang before)
https://github.com/theredcmdcraft/micro/tree/nftables-improvements

I doubt @JoeKar needs help with that. He is a maintainer of micro.

I doubt @JoeKar needs help with that. He is a maintainer of micro.

Oops. I didn`t see the Collaborator Symbol on the right side.

No problem, it was my fault not testing it earlier at work were we've some good examples of nftable rules. It was still on my todo before it was merged.
Ok, anyway...since this merge request is already merged we should create a new one to properly add the fixes into the nftables.yaml.
I suggest to do the following (after the "upstream" is available in your private fork too):

git check nftables-improvements git fetch --all git reset HEAD~1 git stash git reset --hard upstream/master git stash apply git stash add runtime/syntax/nftables.yaml git commit -m "Added Suggested changes" git push --force-with-lease

Create from this resulting branch (nftables-improvements) in your fork a new pull request to to our upstream repository. Sure you can follow your own approach to create a new clean PR.
Currently I've some doubts that the changed symbol and constant.string will work as expected.

No problem, it was my fault not testing it earlier at work were we've some good examples of nftable rules. It was still on my todo before it was merged. Ok, anyway...since this merge request is already merged we should create a new one to properly add the fixes into the nftables.yaml. I suggest to do the following (after the "upstream" is available in your private fork too):

git check nftables-improvements git fetch --all git reset HEAD~1 git stash git reset --hard upstream/master git stash apply git stash add runtime/syntax/nftables.yaml git commit -m "Added Suggested changes" git push --force-with-lease

Create from this resulting branch (nftables-improvements) in your fork a new pull request to to our upstream repository. Sure you can follow your own approach to create a new clean PR. Currently I've some doubts that the changed symbol and constant.string will work as expected.

Did i have to do these steps, when i synced my repo with this repo? I Synced the Fork, and then i created a new branch from the master branch, where i did the changes yesterday. Or can i simply create a new PR with my newly created branch as source. The Target is the micro master branch.

Did i have to do these steps, when i synced my repo with this repo? I Synced the Fork, and then i created a new branch from the master branch, where i did the changes yesterday.

Sure, you can use a fully new/clean branch derived from the synced master to publish your changes within a new PR.

The new PR is #3517

niten94 reviewed Jun 4, 2024

View reviewed changes

runtime/syntax/nftables.yaml Outdated Show resolved Hide resolved

runtime/syntax/nftables.yaml Outdated Show resolved Hide resolved

runtime/syntax/nftables.yaml Outdated Show resolved Hide resolved

theredcmdcraft requested a review from niten94 June 9, 2024 14:06

dmaluka mentioned this pull request Jun 9, 2024

Improve and unify CopyLine, CutLine, DeleteLine, DuplicateLine actions #3335

Merged

niten94 suggested changes Jun 24, 2024

View reviewed changes

runtime/syntax/nftables.yaml Outdated Show resolved Hide resolved

runtime/syntax/nftables.yaml Outdated Show resolved Hide resolved

runtime/syntax/nftables.yaml Outdated Show resolved Hide resolved

runtime/syntax/nftables.yaml Outdated Show resolved Hide resolved

niten94 suggested changes Jul 2, 2024

View reviewed changes

niten94 approved these changes Jul 12, 2024

View reviewed changes

theredcmdcraft force-pushed the master branch from 2f98a83 to e36a20c Compare August 8, 2024 21:39

Create nftables.yaml

9e70322

Created nftables syntax highlighting

theredcmdcraft force-pushed the master branch from 8bbb7a9 to 9e70322 Compare September 8, 2024 13:14

dmaluka merged commit ac73f18 into zyedidia:master Oct 6, 2024

JoeKar reviewed Oct 21, 2024

View reviewed changes

theredcmdcraft mentioned this pull request Oct 22, 2024

Nftables improvements #3517

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Syntax Highlighting for nftables-firewall config file #3325

Added Syntax Highlighting for nftables-firewall config file #3325

theredcmdcraft commented May 30, 2024

niten94 commented Jun 9, 2024

theredcmdcraft commented Jun 9, 2024

theredcmdcraft commented Jun 23, 2024

niten94 left a comment

niten94 commented Jun 24, 2024 •

edited

Loading

theredcmdcraft commented Jun 24, 2024

niten94 commented Jul 2, 2024

theredcmdcraft commented Jul 12, 2024

niten94 commented Jul 12, 2024

theredcmdcraft commented Jul 12, 2024

niten94 commented Jul 12, 2024

theredcmdcraft commented Aug 8, 2024

niten94 commented Aug 10, 2024

theredcmdcraft commented Sep 8, 2024

theredcmdcraft commented Oct 5, 2024

JoeKar left a comment

JoeKar Oct 21, 2024

JoeKar Oct 21, 2024

JoeKar Oct 21, 2024

theredcmdcraft Oct 21, 2024

JoeKar Oct 21, 2024

theredcmdcraft Oct 21, 2024

dmaluka Oct 21, 2024

theredcmdcraft Oct 21, 2024

JoeKar Oct 21, 2024

theredcmdcraft Oct 22, 2024

JoeKar Oct 22, 2024

theredcmdcraft Oct 22, 2024

	- constant.number: "(\\b([1-9][0-9]\|0[0-7]\|0[Xx][0-9A-Fa-f]+\|0[Bb][01]+)([Uu][Ll]?[Ll]?\|[Ll][Ll]?[Uu]?)?\\b)"
	# Decimal Floating Constants
	- constant.number: "(\\b(([0-9][.][0-9]+\|[0-9]+[.][0-9])([Ee][+-]?[0-9]+)?\|[0-9]+[Ee][+-]?[0-9]+)[FfLl]?\\b)"
	# Hexadecimal Floating Constants
	- constant.number: "(\\b0[Xx]([0-9A-Za-z][.][0-9A-Za-z]+\|[0-9A-Za-z]+[.][0-9A-Za-z])[Pp][+-]?[0-9]+[FfLl]?\\b)"

Added Syntax Highlighting for nftables-firewall config file #3325

Added Syntax Highlighting for nftables-firewall config file #3325

Conversation

theredcmdcraft commented May 30, 2024

niten94 commented Jun 9, 2024

theredcmdcraft commented Jun 9, 2024

theredcmdcraft commented Jun 23, 2024

niten94 left a comment

Choose a reason for hiding this comment

niten94 commented Jun 24, 2024 • edited Loading

theredcmdcraft commented Jun 24, 2024

niten94 commented Jul 2, 2024

theredcmdcraft commented Jul 12, 2024

niten94 commented Jul 12, 2024

theredcmdcraft commented Jul 12, 2024

niten94 commented Jul 12, 2024

theredcmdcraft commented Aug 8, 2024

niten94 commented Aug 10, 2024

theredcmdcraft commented Sep 8, 2024

theredcmdcraft commented Oct 5, 2024

JoeKar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niten94 commented Jun 24, 2024 •

edited

Loading