Casbin.NET icon indicating copy to clipboard operation
Casbin.NET copied to clipboard

[BUG] White space is included in policy effect in preview branch

Open AsakusaRinne opened this issue 3 years ago • 6 comments

The environment

In branch preview, windows 11, .NET 6

The description

In the preview branch, the loaded policy effect includes white space, while the PermConstants.PolicyEffect does not.

For instance, we add some white spaces to the basic_model.conf in the examples of unit tests, as shown below.

[request_definition]
r = sub, obj, act

[policy_definition]
p = sub, obj, act 

[policy_effect]
e = some(where (p.eft == allow))    # Before this comment there are some white spaces.

[matchers]
m = r.sub == p.sub && r.obj == p.obj && r.act == p.act

Then the expression of policy effect cannot be matched, which throws an exception.

If it's an unexpected behavior, I would like to solve it soon.

AsakusaRinne avatar Jun 18 '22 17:06 AsakusaRinne

@sagilio @xcaptain @huazhikui

casbin-bot avatar Jun 18 '22 17:06 casbin-bot

@sagilio

hsluoyz avatar Jun 19 '22 02:06 hsluoyz

@AsakusaRinne plz see the supported comment format in Golang, I'm not sure if this inline comment is supported or not.

hsluoyz avatar Jun 19 '22 02:06 hsluoyz

@AsakusaRinne is this issue the same one as: https://github.com/casbin/Casbin.NET/issues/258 ? Why open two duplicated issues?

hsluoyz avatar Jun 19 '22 02:06 hsluoyz

@AsakusaRinne is this issue the same one as: #258 ? Why open two duplicated issues?

Not really, #258 is the lack of process of comment symbol, while this issue is raised because the expression includes white symbol. I agree that they are similar, I would like to solve it with #258 in the same PR if it's indeed an unexpected behavior.

AsakusaRinne avatar Jun 19 '22 04:06 AsakusaRinne

@AsakusaRinne I agree with you, and both are commenting style problems. For this issue my comment is already offered at: https://github.com/casbin/Casbin.NET/pull/262#issuecomment-1159721651 . They can be tackled in one PR.

hsluoyz avatar Jun 19 '22 13:06 hsluoyz

Fixed by #262

sagilio avatar Mar 07 '23 15:03 sagilio