Question: Is there a way to force 2 spaces? about credo

[Lysholm]

@sstern6, @rrrene: I fully agree that semantics checking should be in a separate or multiple separate rules.

I just don't see how we could add a new rule for the kind of indentation and still keep a user friendly interface. It would be really counter intuitive to have a tabs or spaces rule and a 2 space vs 4 space rule and then possibly additional ones in the future. Written naively like the current tabs vs spaces rule you would end up with something that wants incompatible changes on the same line. To avoid that the new code would have to know about the other rule either directly or indirectly, that seems wrong to me.

What i would like to do is create a new rule that checks for arbitrary indentation kind (default to 2 spaces, but can be overridden to any syntax legal indentation), and deprecate the tabs or spaces rule when it's ready. That way we don't break anyone's current config, while maintaining the current usability.

[rrrene]

This will be part of v0.4.0 or v0.5.0. Let's keep this issue open and I will ping you when it's done! 👍

[henryj]

Cool. Thanks.

On Wednesday, 9 March 2016, René Föhring [email protected] wrote:

This will be part of v0.4.0 or v0.5.0. Let's keep this issue open and I
will ping you when it's done! [image: 👍]

—
Reply to this email directly or view it on GitHub
#51 (comment).

[Lysholm]

I see you've released 0.5 recently. Any update on this? If no one's working on it i could take a look.

[rrrene]

Please do. 0.5 was an estimation which did not hold up to reality.

This would be a new check in my opinion, which would check all intendation to be multiples of the base (2 for example). It would have to ignore multi-line strings.

Any other thoughts? /cc @sstern6

[Lysholm]

There's already the consistency check "TabsOrSpaces". I think that could be generalized to check for consistent indentation. What do you think? @rrrene

[rrrene]

The problem with checking for consistent intendation exceeds the responsibility of the TabsOrSpaces check in my book. You can absolutely try to integrate it into the existing check, but I'd imagine that to be harder than creating another check for intendation using a fix amount of spaces.

Please also note that you will also probably not want to deal to deal with semantically correct indentation:

module_names = 
  mod_list 
  |> Credo.Code.Name.full 
    |> List.wrap
    {ast, modules ++ module_names}

where the last two lines are indented inconsistently. Checking this would be a challenging task, I guess.

[sstern6]

IMO I agree with @rrrene this should be a separate rule, including this into TabsOrSpaces will make that rule extremely more complicated and fragile adding this new functionality.

In my experience with working on the indent rule in eslint, dealing with indentation gets increasingly more complicated as you keep growing the rules requirements/functionality(dont let this discourage you from working on it! 😄 ).

That being said, I think if this were to be accepted as a new rule proposal, it should be well scoped to be realistic getting this into the code base with reviews, feedback, etc to be implemented within the next few major releases.

For example, having a rule that is IndentationOfBaseTwo that only checks indentation of 2 spaces and throws an error for anything more or less. I mean i am open to discussion about why we should have multiple options of indentation but I think 2 is pretty standard, unless im missing something.

Then after this has been flushed out and 1.0 has been released we can increase the flexibility of the rule.

Thoughts @rrrene @Lysholm @henryj ???????

Thanks

[rrrene]

I agree on everything but this:

That way we don't break anyone's current config, while maintaining the current usability.

We are explicitly losing usability if we require people to define their preference. This very thing is why consistency checks were thought up in the first place.

But I think this can be beneficial as an optional check for people to utilize who want to do so.

[Lysholm]

@rrrene: I was unclear about what i was referring to there, but i take it you disagree with my proposed default? ("2 spaces") And would rather see the default be the most commonly occurring indentation? If that reading is correct then i fully agree, and that's what I've started working on.

[DieMaske]

Hi guys, just stumbled over this discussion. Is this feature still planned? I would really like to add this rule to my projects!

[rrrene]

@DieMaske we won't implement this in the near future, especially not since elixirfmt seems to become reality: https://summerofcode.withgoogle.com/projects/#5205518009237504

After re-reading this issue's conversation, I think that checking intendation based on a preference like "2 spaces" is a problem that just seems easy on the surface.

I anyone disagrees with me and is willing to provide a working check that handles edge-cases in a PR then I would merge it, but I am unable to sink time into this myself. And, as I said, I would hesitate to implement something like this before the GSoC has concluded.