search

TypeFox / yang-lsp

A Language Server for YANG
http://www.yang-central.org
Apache License 2.0
51 stars 13 forks source link

Fix bad-import-yang-version handling #237

Closed esmasth closed 4 months ago

esmasth commented 5 months ago

It is allowed to import YANG 1.1 modules from YANG 1.0 when importing without revision-date. bad-import-yang-version was flagged even when a YANG 1.1 module was imported by YANG 1.0 module without specifying revision-date and is now fixed.

This fix addresses TypeFox/yang-lsp#228

dhuebner commented 5 months ago

@esmasth Thanks a lot for your contribution! I will re-view and merge your PR this week.

dhuebner commented 5 months ago

@esmasth Btw. you can use addIssue(...) instead of error(...) to make the validation severity configurable. The severity will then be fetched from the settings, or the default defined in io.typefox.yang.validation.IssueCodes will be used.

esmasth commented 4 months ago

@dhuebner Thanks, is it decided then to make these issues configurable? I rather agree with your statement on yang-vscode that it's a potential to break the engine if we allow unambiguously wrong input. warning severity configurability would make sense to me for this code, if we didn't actually fix the bug.

dhuebner commented 4 months ago

@esmasth

is it decided then to make these issues configurable?

No, but we can allow to configure it by the user. I think the user can then decide what to do.

if we didn't actually fix the bug. Your PR fixes the validation, or not?

Do you what to apply the suggestions I made, or should I merge your PR as it is?

esmasth commented 4 months ago

I will do the late evaluation and .empty change as per your good feedback. Just didn't get around to it.

It might be good to have some strictness for MUST/SHALL statement violations in YANG RFC with some consistent policy for the project. Maybe good to discuss that on TypeFox/yang-vscode#40 to conclude?

dhuebner commented 4 months ago

@esmasth Good idea. Let's do it. I'm not using Yang as a developing language, so professional input and discussions a very welcome :)

esmasth commented 4 months ago

Ok with merging this now, if you are.

dhuebner commented 4 months ago

@esmasth Merged. Thanks for contributing again :)