specification icon indicating copy to clipboard operation
specification copied to clipboard

The definition of the indent_size is very confusing

Open mipo256 opened this issue 7 months ago • 7 comments

I think that the definition of the indent_size is very confusing:

   * - ``indent_size``
     - Set to a whole number defining the number of columns used for each
       indentation level and the width of soft tabs (when supported). If this
       equals ``tab``, the ``indent_size`` shall be set to the tab size, which
       should be ``tab_width`` (if specified); else, the tab size set by the
       editor. The values are case-insensitive.

How is it possible that the indent_size which is a numeric field by definition can be equal to literal "tab"?

The second sentence:

If this equals tab, the indent_size shall be set to the tab size, which should be tab_width

How the indent_size can be equal to tab and "shall be set to the tab size" at the same time?

And finally:

The values are case-insensitive

How the positive integer can be case-sensitive in the first place? It does not make sense.

mipo256 avatar May 03 '25 12:05 mipo256

I think we need something better, like (the term "soft tab" will be clarified in this PR #84):

   * - ``indent_size``
     - Set to a whole number defining the number of columns used for each
       indentation level and the width of soft tabs (when supported). If ``indent_size``
       is supposed to be a single hard tab, then the ``indent_size`` shall be set to the
       numerical value of the size equal to ``tab_width`` (if specified); 

CC: @xuhdev @cxw42 @florianb

mipo256 avatar May 03 '25 12:05 mipo256

@mipo256 thanks for your attention to detail! What is the problem to be solved? Are you implementing a plugin or core, and having difficulty doing so?

How is it possible that the indent_size which is a numeric field by definition can be equal to literal "tab"?

Very easily =D . Values in EditorConfig files are untyped! The spec defines no types.

This specific case is in the core tests:

https://github.com/editorconfig/editorconfig-core-test/blob/895b3a65d0d823dbd0acf2bc402376381995d1b1/properties/tab_width_default.in#L9

cxw42 avatar May 11 '25 19:05 cxw42

@cxw42 I think @mipo256 means the language is confusing. The first sentence says "set to a whole number...", but the second sentence says "If this equals tab...". It's a bit confusing in a way that how the whole number can be tab. Someone may misunderstand it as the whole number equal the length of tab.

xuhdev avatar May 11 '25 20:05 xuhdev

Thanks for the clarification! Sure, we could say "set to a whole number or the string tab. If whole number ... . If tab ... .`

cxw42 avatar May 11 '25 22:05 cxw42

Thank you guys for your attention @xuhdev @cxw42

@mipo256 thanks for your attention to detail! What is the problem to be solved? Are you implementing a plugin or core, and having difficulty doing so?

Yes, I'm implementing the maven editorconfig plugin. Rather, I already did.

Very easily =D . Values in EditorConfig files are untyped! The spec defines no types.

It is not specified in the spec. When I implemented a plugin for this spec, and was not clear at all.

@cxw42 I think @mipo256 means the language is confusing. The first sentence says "set to a whole number...", but the second sentence says "If this equals tab...". It's a bit confusing in a way that how the whole number can be tab. Someone may misunderstand it as the whole number equal the length of tab.

That is exactly my point.

mipo256 avatar May 13 '25 06:05 mipo256

   * - ``indent_size``
     - Set either to a whole number defining the number of columns used for each
       indentation level and the width of a single soft tab (when supported). Alternatively, 
       the ``indent_size` can be set onto a literal value "tab" (case-insensitive). In this case, the eventual 
       indentation level must be equal to numerical amount of columns that comprise a 
       single hard tab. 

Concerning everything said above, I have came up with this definition. What are your thoughts on this, @xuhdev @cxw42 ?

mipo256 avatar May 13 '25 07:05 mipo256

I agree with the general idea, we can polish the grammar and details together in a PR.

xuhdev avatar May 14 '25 07:05 xuhdev