casbin / Casbin.NET

An authorization library that supports access control models like ACL, RBAC, ABAC in .NET (C#)
https://casbin.org
Apache License 2.0
1.13k stars 110 forks source link

fix: Enable the comment in model config files. #262

Closed AsakusaRinne closed 1 year ago

AsakusaRinne commented 2 years ago

Fix: #258, #263

What does it solve

It add support for writing comments in model config files.

What does it change

It add a new const variable to indicate the comment symbol '#' and remove the part after comment symbol in the string in AddDef.

casbin-bot commented 2 years ago

@sagilio @xcaptain @huazhikui please review

casbin-bot commented 2 years ago

@sagilio @xcaptain @huazhikui please review

AsakusaRinne commented 2 years ago

@sagilio The comment itself is not affected by the model config. However, model config contents are necessary to test it. So I just add comments to an existing model config test file. Do you think that writing a dependent file to test the comments is better, or just keeping it is enough?

hsluoyz commented 2 years ago

@sagilio

sagilio commented 2 years ago

@sagilio The comment itself is not affected by the model config. However, model config contents are necessary to test it. So I just add comments to an existing model config test file. Do you think that writing a dependent file to test the comments is better, or just keeping it is enough?

I think we need to write new example files and tests for the comments test, and the test also needs to cover the entire line of comments.

This will also more helpful to others format support in future.

hsluoyz commented 2 years ago

@AsakusaRinne

  1. Don't mess with the existing models. Try to fork a new model from RBAC and test all your supported comments on that file only.
  2. Make sure to align with the Golang Casbin, you should NOT create a comment style that are not compatible with Golang Casbin's model. If you are doing so, try to get help from the community to let Golang Casbin support it first (or make PR to Golang Casbin by yourself if you know Go). Finally all Casbin implementations will support the SAME commenting style and Golang Casbin should be the first to support it, then port to others.
AsakusaRinne commented 2 years ago

@AsakusaRinne

  1. Don't mess with the existing models. Try to fork a new model from RBAC and test all your supported comments on that file only.
  2. Make sure to align with the Golang Casbin, you should NOT create a comment style that are not compatible with Golang Casbin's model. If you are doing so, try to get help from the community to let Golang Casbin support it first (or make PR to Golang Casbin by yourself if you know Go). Finally all Casbin implementations will support the SAME commenting style and Golang Casbin should be the first to support it, then port to others.

OK, I got it, I have signed this PR as WIP and will complete the tips above before requesting a merge.

sagilio commented 2 years ago

@AsakusaRinne

  1. Don't mess with the existing models. Try to fork a new model from RBAC and test all your supported comments on that file only.
  2. Make sure to align with the Golang Casbin, you should NOT create a comment style that are not compatible with Golang Casbin's model. If you are doing so, try to get help from the community to let Golang Casbin support it first (or make PR to Golang Casbin by yourself if you know Go). Finally all Casbin implementations will support the SAME commenting style and Golang Casbin should be the first to support it, then port to others.

@hsluoyz golang version also supports the inline comment: https://github.com/casbin/casbin/blob/5225b867429740e5ffdb69a124c3890f1a66a4f9/examples/comment_model.conf#L1-L12

hsluoyz commented 1 year ago

@AsakusaRinne fix:

image

AsakusaRinne commented 1 year ago

@sagilio Do you think we still need this fix?

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 2.0.0-preview.5 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

github-actions[bot] commented 11 months ago

:tada: This PR is included in version 2.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

github-actions[bot] commented 11 months ago

:tada: This PR is included in version 2.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: