TypeFox / yang-lsp

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

Yang-lsp serialization indentation with 2 spaces #226

Closed RimShao closed 9 months ago

RimShao commented 1 year ago

Hi Miro, Dennis

Hope you are doing well.

Request yang-lsp serialization to generate yang files keeps indentation with 2 spaces, we project have such design rule on it.

Can you think about that ? Thanks.

dhuebner commented 1 year ago

@RimShao If I recall it correctly we have a 4-space indentation in yang-lsp. You would like to have 2-spaces is that your request? If so, we could use yang.settings to control indentation string as setting.

dhuebner commented 1 year ago

See also: https://github.com/TypeFox/yang-vscode/issues/70

RimShao commented 1 year ago

@dhuebner

Thanks the information.

do you have a example how to configure for n-spaces in yang.settings, I tried few ways as below however no luck

"yang": {
                "insertSpaces": true,
                "tabSize": 2
        }

    "[yang]": {
        "editor.insertSpaces": true,
        "editor.tabSize": 2
    }
dhuebner commented 1 year ago

@RimShao I need first investigate if and how the vs-code settings are propagated to the yang-lsp. But looking into the https://github.com/TypeFox/yang-vscode/issues/70 , which is still open, it doesn't seem to work.

@spoenemann I think we should address this one soon as some people are already having the issue. WDYT?

spoenemann commented 1 year ago

Yes, we could start this in March if it doesn't take too long

RimShao commented 1 year ago

Hi @dhuebner

Do you have time to look into this question ?

Thanks.

dhuebner commented 1 year ago

@RimShao I will be able to look into it in May probably.

dhuebner commented 9 months ago

@RimShao The default indent string in yang-lsp is four spaces. Should we change it to two spaces globally? Or are you going to configure it on user, workspace or project level using yang.settings?

dhuebner commented 9 months ago

@RimShao When working with IDE (e.g. vs-code) the IDE's formatting settings bypassed with the format-request are now used. This can be configured in "Settings" under "Editor: Tab Size". It also can be configured per language. For yang in setting.json as example:

"[yang]": {
      "editor.tabSize": 5,
    },

If no IDE settings are provided the yang.settings file will be used to read the indentation property. See https://github.com/TypeFox/yang-lsp/blob/master/docs/Settings.md.

If no file is present the default indentation is used. This is currently four spaces.

The yang-lsp 0.7.4-SNAPSHOT contains related fixes.

RimShao commented 9 months ago

Hi @dhuebner

Thanks the update.

Where is yang-lsp 0.7.4-SNAPSHOT? neither in oss.sonatype.org nor in mvnrepository.

BR// Rim

dhuebner commented 9 months ago

@RimShao

Here it is available, look the version from 03.11.2023 https://oss.sonatype.org/content/repositories/snapshots/io/typefox/yang/io.typefox.yang/0.7.4-SNAPSHOT/

RimShao commented 9 months ago

@dhuebner

Thanks and will try out it.

BR// Rim

RimShao commented 9 months ago

Hi @dhuebner

I tried the 0.7.4-SNAPSHOT/, and has no luck.

I actually try to use both below contents into yang.settings, either in root project and ~/.yang/yang.settings.

Please also kindly know that I running with standalone executable jar utility.

"[yang]": {
      "editor.tabSize": 5
    }
"yang": {
      "editor.tabSize": 5
    }
dhuebner commented 9 months ago

@RimShao

When using yang.settings the property is called indentation. Note it is not the vs-code settings format, but standalone. See https://github.com/TypeFox/yang-lsp/blob/master/docs/Settings.md

So for two spaces you will need this json content as ~/.yang/yang.settings.

{
   "indentation": "  "
}

So the value of the property indentation is a string that will be used as indent.

RimShao commented 9 months ago

Hi @dhuebner

Thanks the update. Then I tried accordingly over your update, still have no luck.

{
   "indentation": "  "
}

The above ~/.yang/yang.settings actually running into below error:

23-11-07 15:04:03.809 ERROR JsonFileBasedPreferenceValues:61 - Error reading settings 'C:\Users\xyz\.yang\yang.settings' : com.google.gson.stream.MalformedJsonException: Expected name at line 2 column 2 path $.

Then I try below with ~/.yang/yang.settings, either have no luck

"yang" : {
   "indentation": "  "
}

"diagnostic" : {
        "indentation": "  "
}

By the way, I don't see you have test case accordingly with this indentation commit, probably you shall have a local try. Thanks.

dhuebner commented 9 months ago

@RimShao I've added a tests that parses

{
   "indentation": "  "
}

I also changed my ~/.yang/yang.settings accordantly and checked standalone tests. They started failing because of the changed indentation string. Could you see what is the (Line:2, Col: 2) character is in your yang.settings? Maybe some pseudo space char?

RimShao commented 9 months ago

Hi @dhuebner

I tried again, now ~/.yang/yang.settings works.

{
   "indentation": "  "
}

Now I would like to include this fix into my existing yang.settings in root of released standalone jar. do you know how to support this?

This is my existing root yang.settings


{
        "extension" : {
                "classpath" : ".",
                "validators" : "com.xyz.yang.extensions.validation.xyzYangRuleValidator:com.xyz.yang.extensions.validation.xyzExtensionsValidator"
        },

        "diagnostic" : {
                "XYZ_YANG_INFO" : "info", "XYZ_YANG_WARNING" : "warning"
        }
}

I tried below for indentation, it doesn't work


{
        "extension" : {
                "classpath" : ".",
                "validators" : "com.xyz.yang.extensions.validation.xyzYangRuleValidator:com.xyz.yang.extensions.validation.xyzExtensionsValidator"
        },

        "diagnostic" : {
                "XYZ_YANG_INFO" : "info", "XYZ_YANG_WARNING" : "warning", 
                "indentation": "  "
        }
}

Then I tried below for indentation, either it doesn't work


{
        "indentation": "  ",
        "extension" : {
                "classpath" : ".",
                "validators" : "com.xyz.yang.extensions.validation.xyzYangRuleValidator:com.xyz.yang.extensions.validation.xyzExtensionsValidator"
        },

        "diagnostic" : {
                "XYZ_YANG_INFO" : "info", "XYZ_YANG_WARNING" : "warning"
        }
}
RimShao commented 9 months ago

It looks yang-lsp doesn't pick yang.settings from root of a project as documented in https://github.com/TypeFox/yang-lsp/blob/master/docs/Settings.md, or anything I missed here ?

dhuebner commented 9 months ago

@RimShao I'm not sure how is your runtime is configured. Or better to say, what should the "project" be if one uses yang-lsp standalone? The working directory? I think the IProjectConfigProvider will be missing and in io.typefox.yang.settings.PreferenceValuesProvider.createPreferenceValues(URI) only the user settings can be used. I don't think it is clearly defined currently for the standalone case. We maybe should have a meeting to find out how it should work.

RimShao commented 9 months ago

Hi @dhuebner

Thanks the information.

Now I have fixed loading specified yang.settings through ProjectConfigAdapter.install, everything works well now.

Then next step how can I get the fix, will you publish new release 0.7.4?

Regards Rim

dhuebner commented 9 months ago

@RimShao Glad to hear it works for you! I will prepare a release today. It also makes sense to release the yang-vscode extension.

dhuebner commented 9 months ago

Released with v0.7.4

https://repo1.maven.org/maven2/io/typefox/yang/io.typefox.yang/0.7.4/

RimShao commented 9 months ago

Thanks a lot. @dhuebner