Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use tabs for indentation instead of spaces #270

Open
migjc opened this issue Jun 14, 2023 · 3 comments
Open

Use tabs for indentation instead of spaces #270

migjc opened this issue Jun 14, 2023 · 3 comments
Labels
component: core Affects the core infrastructure component: lexer Affects the lexer component: parser Affects the parser difficulty: extreme This change requires significant language/algorithm/architecture design tool: mh_style Affects the style checker tool

Comments

@migjc
Copy link

migjc commented Jun 14, 2023

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

@florianschanda
Copy link
Owner

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 florianschanda added component: parser Affects the parser component: lexer Affects the lexer tool: mh_style Affects the style checker tool component: core Affects the core infrastructure difficulty: extreme This change requires significant language/algorithm/architecture design labels Jun 15, 2023
@migjc
Copy link
Author

migjc commented Jun 19, 2023

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...

@florianschanda
Copy link
Owner

florianschanda commented Jun 20, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: core Affects the core infrastructure component: lexer Affects the lexer component: parser Affects the parser difficulty: extreme This change requires significant language/algorithm/architecture design tool: mh_style Affects the style checker tool
Projects
None yet
Development

No branches or pull requests

2 participants