robotframework-robocop icon indicating copy to clipboard operation
robotframework-robocop copied to clipboard

Configurable mixed tabs and spaces

Open adrszad opened this issue 1 year ago • 4 comments

I observe this rule is often confusing for the users, especially as no line number is given. It could be a project setting if we use spaces or tabs, and then each occurance of wrong separator could be logged separately with line number.

adrszad avatar May 17 '23 10:05 adrszad

Hey @adrszad, thanks for creating the issue and sorry for late reply.

I think it's a valid point that it may be configurable. Would you see it as a rework of the rule W1006 mixed-tabs-and-spaces in a form of a new rule like inconsistent-separators (or some different name) that can take a parameter like separator with possible values tab, space, or mixed and then report only when the rule is violated?

I recall we have discussed it before with @bhirsz and we are aware that opinions are divided whether it's better to use spaces or tabs. That's the reason we decided that maybe it's better to not enforce one or the other, and only detect the case when both of them are mixed within one file.

Personally, I would like Robocop to not enforce tabs or spaces and let people decide what they want to use, and not report the issue until it is explicitly set by the user that one or the other is required. So it'd be kind of a disabled rule until it's explicitly used or configured.

What do you think?

As a sidenote, it may be a good candidate for the community rule (which we plan to add in the future), but I think it may also be included as a built-in rule, because I see it as a beneficial.

mnojek avatar May 31 '23 12:05 mnojek

@bhirsz @adrszad Any thoughts here? I don't have a strong opinion, so it would be great to discuss what to do with it before the next steps. Feel free to respond whenever you have time. Let's talk 🙂

mnojek avatar Jun 15 '23 13:06 mnojek

I would like Robocop to not enforce tabs or spaces and let people decide what they want to use

It's actually working like this right now. If user only uses spaces, or tabs, mixed-tabs-and-spaces will not be reported. I'm fine with configuring it so it's possible to use fixed value (report every occurence of tab or space even if it's consistent). What we're missing is reporting on every occurence - which can over clutter log bug is really helpful in catching small typos in larger files.

So it could:

mixed: by default, only check if there are mixed separators space or tab: validate that only space/tab is used within file

bhirsz avatar Jun 15 '23 13:06 bhirsz

@bhirsz Yes, I know it works like this, but user has no control over what is used, because the rule only reports inconsistency. But it would be good to give the user a way to enforce tabs or spaces for each file.

The only concern I have is how should we introduce it into Robocop, since we already have a rule W1006 mixed-tabs-and-spaces. Having it configurable with new parameter separator and values mixed, tab, space makes the name of the rule no longer applicable. That's why I think we should add a new rule called inconsistent-separators.

Though, there is another issue now - if we want this rule to report on every place where the other separator is used, for the separator=mixed option, it won't be possible to indicate if it should report on all spaces or all tabs. For tab or space options it's trivial, because it would report on the opposite value.

So maybe we should keep also the old rule that will check if the separators are inconsistent in the file and report only once? This way we will have one rule to report if they are mixed, and one to report the exact place where the unwanted separator is used.

There is also a variant to add one more param to the new rule (or even old rule) and allow the users to decide whether it should report once or each occurrence, but I'm not a fan of this solution, since this affects too much how one rule may work.

What do you think?

mnojek avatar Jun 15 '23 20:06 mnojek