miss_hit icon indicating copy to clipboard operation
miss_hit copied to clipboard

Use tabs for indentation instead of spaces

Open migjc opened this issue 1 year ago • 3 comments

I would like miss_hit to be able to indent the code with tabs instead of spaces, and this option to

Environment: matlab r2022b

MISS_HIT component affected

  • Style checker
  • Lexer
  • Documentation

Would this be a feature that it would be ok to integrate?

I have already played with the code and I am able to use tabs succesfully, although there is no columnar alignment on functions, matrices, etc. I just increase by 1 the previous indent. I think this is consistent and makes sense.

Based on what I have understood from the code, the remaining bits are:

  • Clean the code a bit.
  • Add the configuration option, something like "indentation character", or "indent with".
  • Make my code changes to run only if the option to indent with tabs is enabled
  • Modify the error logic so we mark spaces as errors when using tabs and the other way around.
  • Add relevant unit tests.
  • Anything else?

I am more than happy to complete the work and do a pull request. Any advice would be appreciated

I am attaching here the changes as text files with the diffs.

m_lexer.diff.txt mh_style.diff.txt

migjc avatar Jun 14 '23 19:06 migjc

I have to say I am not keen. I have put considerable effort into eliminating tabs, and pretty much based everything around that.

There are two reasons for this:

  • any coding standard I am aware of for MATLAB says to use spaces instead of tabs
  • it simplifies things internally

I am also not keen to test this: we'd have to run the entire test-suite in duplicate and hand-review the output; because it is such a fundamental change. And I have had correctness-affecting bugs in MH before due to messing up whitespace, because MATLAB is such a dumb language where whitespace can have semantic changing effects.

That said I will not stand in the way of a well crafted PR, but I have to be honest with you: I will need a fair bit of convincing to merge it here. Expect a long iteration cycle with a ton of tedious comments from my side :)

florianschanda avatar Jun 15 '23 09:06 florianschanda

Hi Florian,

Thank you for your honest reply. To be frank, it is not totally unexpected. Spaces vs tabs is such a fundamental decision that if you had wanted to give the option, it would have been built in from the beginning. The lexer engine is clearly designed for spaces / character columns and it does its job well.

For me, tabs vs spaces is a way to express intent: if I indent, I use tabs; for anything else, I use spaces. This also makes easier for the developer to customize the way code is read (changing visualization of chars per tab on the editor / IDE) without changing what is being written in the source file (which will always be a tab). I have worked with legacy code where the same file had commits with different spaces settings (some 3, some 4, and some even 2!). That was horrible to follow when doing diffs. A tab is a tab and will always be displayed with the same way.

However, I understand this is personal, and there are both reasons for and against. Coming from outside, imposing anything at all into a project like this would be mad. I will therefore keep my work in a branch for now. When I think I have something good enough, I will start the conversation of the merge / pull request process if you are happy to have a look. I will do my best not to touch the engine and the defaults will always involve to maintain the current functionality.

In the meantime, I will keep merging changes from the main so the branch does not become obsolete. I will raise any issues I may find in the tool (if any).

It is funny that you mention Matlab coding standards, on a previous company I did some research on the area, and never found any good or widely accepted one. Could you please send a couple of links if they are public? In the case I mentioned, I ended up writing our Matlab coding standard based on the C++ and Java ones. Obviously a lot was not really usable and a fair amount had to be rewritten. This was about 10 years ago and just kept using what I wrote, updating things with the changes in the Matlab language. Right now, I would have probably started with a python one to be honest...

migjc avatar Jun 19 '23 14:06 migjc

For me, tabs vs spaces is a way to express intent: if I indent, I use tabs; for anything else, I use spaces.

Back when I was much younger I actually had 100% the same view and I still think it is valid. Since then I've become cynical as I have worked in so many industrial projects in different areas.

I now think that having a code formatter that enforces style in a pre-commit / pre-merge check kinda dodges the issues effectively, since that way everyone has it identical. Of course having something that also works for tabs would not invalidate this, but nevertheless the inconsistency issue is mostly resolved.

It is funny that you mention Matlab coding standards, on a previous company I did some research on the area, and never found any good or widely accepted one. Could you please send a couple of links if they are public?

I am not aware of any public ones, but then again I am not really a MATLAB programmer. I wrote this tool out of desperation because I've worked for companies where MATLAB was widely used but the style was all over the place + none of the other tools worked in all cases. :)

I think in terms of public ones, there is this one: https://sites.google.com/site/matlabstyleguidelines/home

I am not sure how much of MISS_HIT's default config implements it though.

florianschanda avatar Jun 20 '23 07:06 florianschanda