Errors in CONFIGURATION.md? #264

Activity

1. krassowski commented on Sep 12, 2022

   

   krassowski

   on Sep 12, 2022

   Member

   More actions

   Thanks for taking a close look. I did not do not anything wrong here, but we could improve CONFIGURATION.md by explaining what null means in this context and make pylsp.plugins.pycodestyle.maxLineLength and integer:

   • pylsp.plugins.flake8.hangClosing, pylsp.plugins.flake8.select, pylsp.plugins.pycodestyle.hangClosing, pylsp.plugins.pydocstyle.convention, pylsp.rope.ropeFolder: Generally for third party tools like flake8, null means that we do not set the value and therefore use the tool's default; the default might change between versions of flake8, pycodestyle and may be different based on configuration sources and environment of the user. Therefore null is different from None. This could be explicitly mentioned in CONFIGURATION.md - would you like to contribute?
   • pylsp.plugins.flake8.maxLineLength: integer is correct; how could a line be 80.5 character?
   • pylsp.plugins.pycodestyle.maxLineLength as per above should be changed to integer
   • pylsp.plugins.flake8.indentSize and pylsp.plugins.pycodestyle.indentSize again, can you enter three and a half spaces? I am pretty sure integer is correct; someone could even restrict it to positive integer but it is more difficult to express in underlying schema
2. doolio commented on Sep 13, 2022

   

   doolio

   on Sep 13, 2022

   ContributorAuthor

   More actions

   Generally for third party tools like flake8, null means that we do not set the value and therefore use the tool's default; the default might change between versions of flake8, pycodestyle and may be different based on configuration sources and environment of the user. Therefore null is different from None. This could be explicitly mentioned in CONFIGURATION.md - would you like to contribute?

   I see. That make sense. I'd be happy to update the configuration file but I do so by updating pylsp/config/schema.json, right?

   integer is correct; how could a line be 80.5 character?
   again, can you enter three and a half spaces?

   Agreed. However, I had assumed it should be number as I thought that the JSON datatype was number to cover both integer and floating point number types. Is that not the case? I'm only starting to learn how to programme (starting with python!) so eager to learn.
3. krassowski commented on Sep 13, 2022

   

   krassowski

   on Sep 13, 2022

   Member

   More actions

   I'd be happy to update the configuration file but I do so by updating pylsp/config/schema.json, right?

   Yes, that is preferred as it will then be in both schema.json and CONFIGURATION.md (after regenerating it). You could update the description field to add it above the table:

   python-lsp-server/pylsp/config/schema.json

   Line 4 in 6501e9e

   "description": "This server can be configured using `workspace/didChangeConfiguration` method. Each configuration option is described below:",

   Alternatively it could be added below the table. We implement it by by adding a comment field in schema.json like "$comment": "extra description for our use only" and then adding it below the table in convert_schema function:

   python-lsp-server/scripts/jsonschema2md.py

   Lines 49 to 73 in 6501e9e

   	def convert_schema(schema: dict, source: str = None) -> str:
   	lines = [
   	f"# {schema['title']}",
   	schema["description"],
   	"",
   	"| **Configuration Key** | **Type** | **Description** | **Default** ",
   	"|----|----|----|----|",
   	]
   	for key, prop in schema["properties"].items():
   	description = prop.get("description", "")
   	default = json.dumps(prop.get("default", ""))
   	lines.append(
   	f"| `{key}` | {describe_type(prop)} | {description} | `{default}` |"
   	)
   	if source:
   	lines.append(
   	f"\nThis documentation was generated from `{source}`."
   	" Please do not edit this file directly."
   	)
   	# ensure empty line at the end
   	lines.append("")
   	return "\n".join(lines)

   I had assumed it should be number as I thought that the JSON datatype was number to cover both integer and floating point number types. Is that not the case?

   JSON schema can encode much more than the JSON data types can hold. "Understanding JSON Schema" is probably a good place to start https://json-schema.org/understanding-json-schema/reference/numeric.html#integer
4. doolio commented on Sep 13, 2022

   

   doolio

   on Sep 13, 2022

   Last edited by doolio

   ContributorAuthor

   More actions

   Thanks for the link. It is clearer now.

   Should pylsp.plugins.jedi_completion.resolve_at_most and pylsp.plugins.mccabe.threshold be of type integer too for consistency with the other integer datatypes in this table? And by the same argument should the default values of pylsp.plugins.pycodestyle.select and pylsp.plugins.pydocstyle.select be set to null for consistency with pylsp.plugins.flake8.select?

   [Aside: Is there any plans to include other third party plugins settings in schema.json. I'm thinking of python-lsp-black, pyls-isort (which hopefully is renamed to python-lsp-isort) etc. so they too can be passed by the client to the server?]

   Edit: Added another setting to a query.
5. krassowski commented on Sep 13, 2022

   

   krassowski

   on Sep 13, 2022

   Member

   More actions

   Should pylsp.plugins.jedi_completion.resolve_at_most and pylsp.plugins.mccabe.threshold be of type integer too for consistency with the other integer datatypes in this table? And by the same argument should the default value of pylsp.plugins.pydocstyle.select be set to null for consistency with pylsp.plugins.flake8.select?

   Yes, these suggestions sound good to me.

   [Aside: Is there any plans to include other third party plugins settings in schema.json. I'm thinking of python-lsp-black, pyls-isort (which hopefully is renamed to python-lsp-isort) etc. so they too can be passed by the client to the server?]

   I think we could encourage plugins which are in separate repositories to add their schema in their repositories; we could make re-uisng the script for converting schema to markdown file easier, but I don't think that we should add those to schema.json in this repository (due to high risk of user confusion, changes between versions, what happens if someone forks the plugin and now you want to use a fork, etc).
6. doolio commented on Sep 14, 2022

   

   doolio

   on Sep 14, 2022

   ContributorAuthor

   More actions

   but I don't think that we should add those to schema.json in this repository (due to high risk of user confusion, changes between versions, what happens if someone forks the plugin and now you want to use a fork, etc).

   OK, I understand. I did not appreciate such concerns.
7. ccordoba12

   added this to the v1.6.0 milestone on Sep 14, 2022
8. ccordoba12

   closed this as completedin #267on Sep 27, 2022