Igor Programming Tool (IPT) for formatting and linting your code

Thank you for this nice tool, which I already tried out.

I don't know whether it is OK to post feedback here, but a couple comments for now:

- First, I would like to note that ANSI escape code parsing is apparently switched off by default on windows which makes the output from this tool near unreadable. It took me quite some head-scratching to get this to work, but I followed this guide in the end:

https://stackoverflow.com/questions/51680709/colored-text-output-in-pow…

It would help to have a short tutorial for different command-line tools and / or an option to switch off formatted output (sorry if this already exists, but I couldn't find it)

- During installation, there is an option to create a desktop shortcut, but this does nothing apparently. I don't know what this would be useful for in any case since it is a command-line tool.

- Apparently, ipt lint and ipt lint -i shows different output for my files (the latter showing MUCH more warnings). I guess this is intended, but I don't get it. Does the -i flag include a setting for a more limited set of checks?

> I don't know whether it is OK to post feedback here, but a couple comments for now

Totally fine for me.

> First, I would like to note that ANSI escape code parsing is apparently switched off by default on windows which makes the output
> from this tool near unreadable. It took me quite some head-scratching to get this to work, but I followed this guide in the end:
> https://stackoverflow.com/questions/51680709/colored-text-output-in-pow…
> It would help to have a short tutorial for different command-line tools and / or an option to switch off formatted output (sorry if this already exists, but I couldn't find it)

We don't do any autodetection if ANSI escape codes are supported. You can use the `--color=no` switch to turn it off. I've made a note to document that. `ipt --help` has all the documentation for the top-level switches and `ipt <cmd> --help` for cmd.

> During installation, there is an option to create a desktop shortcut, but this does nothing apparently. I don't know what this would be useful for in any case since it is a command-line tool.

I've made a note and will followup.

> - Apparently, ipt lint and ipt lint -i shows different output for my files (the latter showing MUCH more warnings). I guess this is intended, but I don't get it. Does the -i flag include a setting for a more limited set of checks?

I'm suprised to hear that `ipt lint -i` shows more warnings. I would have expected the opposite as `-i` implies `--fix` which fixes linting warnings/errors instead of only warning. Do you have an MWE?

Thank you for the quick reply. I meant that 'ipt lint -i' shows much less warnings than 'ipt lint'. But the file itself is never changed in either case, so I wonder what -i is for then. I'll attach a test file. With the latter command I get two warnings and with the former about 6 warnings as output. If I understand you correctly, some warnings do not appear because they are apparently fixed (without actually being fixed in my case). Note that the file is properly formatted and overwritten via ipt format -i.

Regarding the formatting, I personally am not so fond of (see also the testfile before formatting):

- inline comments getting 'unaligned'. I have usually an aligned column for comments with enough visual distance, which I find much easier to read than comments tacked directly behind code.

- tabs for panel control elements being replaced without aligning equal signs (pos={}, size={} etc.). I have panel code also heavily aligned to make the code easily readable.

- equal signs of #pragma not being aligned

The latter two I can 'fix' myself, e.g., by setting an IPT_FORMAT_OFF there. But I am not willing to go through all my hundred comments to realign them after formatting, so no use for this particular feature for me now. It would be great to have a switch for 'leave comment indentation alone'. (Yes, I am aware that this kind of commenting might be considered bad style, but I want comments to sit out of the way and not in-between actual code if I can avoid it).

> If I understand you correctly, some warnings do not appear because they are apparently fixed (without actually being fixed in my case).
 

That's definilty odd. We'll have a look.

> - inline comments getting 'unaligned'. I have usually an aligned column for comments with enough visual distance, which I find much easier to read than comments tacked directly behind code.

That's getting into the taste area. I actually prefer comments in their own lines instead of trailing comments. But that is not something ipt can fix. Now of course we could a flag to keep comments, but until now we tried very hard to not have any knobs for the formatter. The reason is that the complexity of testing and documentation explodes with knobs. A bad example is clang-format (formatter for C++) where you even have tools like https://clang-format-configurator.site/ to help with writing the config file.

> - tabs for panel control elements being replaced without aligning equal signs (pos={}, size={} etc.). I have panel code also heavily aligned to make the code easily readable.

We try to always use recreation macros where the macro code is stored in separate files. So I rarely look at GUI code like that. I'll make a note.

> - equal signs of #pragma not being aligned

I need to think about this one.

Hello, I am one of the developers of this tool. First of all, I appreciate your feedback.

> - Apparently, ipt lint and ipt lint -i shows different output for my files (the latter showing MUCH more warnings). I guess this is intended, but I don't get it. Does the -i flag include a setting for a more limited set of checks?

I looked into the details. Apparently, if we have at least one warning or error for that specific file, nothing changes. We always try to have no warnings in our code, so this bug went unnoticed until now.

> First, I would like to note that ANSI escape code parsing is apparently switched off by default on windows which makes the output from this tool near unreadable.

Interesting, because on my Windows it worked right out of the box, so I didn't think much about it. But I use the modern Windows Terminal and VS code all the time. We can update our documentation to highlight this problem. The solution is to add `--color=no` to your arguments.

Thank you for the follow-up. Regarding the ANSI support, it may be that you have set everything up a long time ago to work with the terminal. I can have a look at another PC, but it initially didn't work ... neither with the terminal nor with Powershell both in ver. 5.1 or 7.5. I just tested it, and color=no works under these circumstances at least.

Another small thing: When there is a warning about a switch statement, the parser seems to include either the \r or \n or something else in the output. See the attached image.

In case a line termination (\r, \n or \r\n) is part of the code with error, we print a replacement character ⏎ [1] to the output. This is only visible if Windows is setup to use Unicode, which is unfortunately not the default and requires administrative changes [2]. I'll create an internal issue, so we can fix this and add some documentation.

[1]: https://www.compart.com/en/unicode/U+23CE

[2]: https://superuser.com/a/1435645

We fixed the ipt lint -i part. It may report some errors that cannot be fixed automatically, but it will always write the changes to the output file.

We also aligned the #pragmas at the top of the file.

To get the new ipt, download the latest release from our website. There is no update on winget yet.

Thank you for the update. Two comments from my side:

- I wonder how the list of warnings for linting is sorted. It seems to be roughly based on line number but not completely (sometimes it shows errors on a much earlier line before increasing again)?

- The ReadabilityMissingParenthesis warning produces two lines of output, one of them only the word 'here'; see the attachment.

> I wonder how the list of warnings for linting is sorted. It seems to be roughly based on line number but not completely (sometimes it shows errors on a much earlier line before increasing again)?

This order depends on certain conditions but is always deterministic. It is primarily influenced by the line numbers, though each rule that creates these warnings can also affect it. We have not added any logic to sort them by line. (Maybe we will.)

> The ReadabilityMissingParenthesis warning produces two lines of output, one of them only the word 'here'; see the attachment.

That's an interesting regression and the output should be different. I will look into it tomorrow.

> The ReadabilityMissingParenthesis warning produces two lines of output, one of them only the word 'here'; see the attachment.

This has been fixed and the newest ipt version (only available at our website) will create a correct error log.

The order of the errors is kept, but if you think a special one is needed, just write to me. I can also add a longer description to the documentation that explains the order.

Thank you for the update. The linter seems to output identical messages multiple times in some cases. See the attached files. I also think I have seen a case where there is no line break in-between two warning outputs (i.e., the messages are directly stitched together and the alignment is all off), but I can't seem to reproduce it at the moment.

Otherwise it works fine for me and I have already updated all my code to produce no warnings with the linter (while switching off three rules I personally do not want to follow strictly).

> The linter seems to output identical messages multiple times in some cases. See the attached files. I also think I have seen a case where there is no line break in-between two warning outputs (i.e., the messages are directly stitched together and the alignment is all off), but I can't seem to reproduce it at the moment.

Max is already looking into that.

> Otherwise it works fine for me and I have already updated all my code to produce no warnings with the linter (while switching off three rules I personally do not want to follow strictly).

Nice to hear that the linter works for you. Out of curiosity can you share which linter rules you have disabled?

Sure, I disable:

• CodeStyleDefaultPragmas

While this is no error, I don't want to use rtFunctionErrors=1. I am working completely alone on about 10 projects, and already debugging the usual code is a monumental task. I simply cannot afford to look for all these edge cases. The manual is very vague anyway about which errors to expect here.

• ReadabilityOneLineVariableInitialization

While I see the point, I don't want to follow this rule after all. For one, I often initialize the variables upon creating them and think having this on separate lines is a waste of space:

variable var1 = 0, var2 = 0, var3 = 0
string str1 = "", str2 = "", str3 = ""

Maybe it could be an exception to the rule that there is no warning if a string is followed by [= ""] only and a variable is followed by [= 0]. This would not discourage such initialization of variables.

Also, there are occasions where combining statements is better for readability (imho):

variable rows = DimSize(  data,0), cols = DimSize(  data,1), layr = DimSize(  data,2)
variable xoff = DimOffset(data,0), yoff = DimOffset(data,1), zoff = DimOffset(data,2)
variable xdel = DimDelta( data,0), ydel = DimDelta( data,1), zdel = DimDelta( data,2)

Yes, I could also spend 9 rows for above code, and maybe I will do that some day. Who knows...

Initially, I also switched off BugproneMissingSwitchDefaultClause, but in the end I decided to follow the rule, even if that meant just inserting many empty 'default' statements into the code.

We are currently in the final stages before the next release, which will include a fix for the duplicate log lines that you reported.

This issue was caused by a minor bug in the rule's internal state machine, which caused the rule to match multiple times.

Thank you for the new version. I am playing around with the formatting lately. While I find most changes helpful, it would be great if there was a list of formatting rules and an --exclude flag for switching some of the off. As mentioned earlier, I don't find it helpful that formatting is messing with my tabbing for alignment, especially for panel code and inline comments. I could work with IPT_FORMAT_OFF in some cases, but having an exclude list would be even better.

Also, I found some possible contradiction in the coding conventions. Wouldn't the following recommendation trigger a ReadabilityElseAfterReturn warning:

if(someCondition)
    // code
    return 0
else
    // code
    return 1
endif

Hi Stephan, thanks for trying out the latest version.

Currently, there is no option to turn on or of certain formatting "rules" or to configure them. In fact there is no formatting "rule" at all. We read the whole Igor source file into an abstract syntax tree (AST) [1]. You can imagine this, as a tree for which each node is some part of the original source code. Any original formatting is lost at this time. Later we print this AST using our own rules.

Linting works a bit different to printing. We have a set of state machines [2], that operate on the given AST and searches for specific patterns. Later is some code executed on found patterns and the AST might be transformed. A linting rule is just a name of a state machine, which can be enabled or disabled.

Our printing do not have any configuration for now, because this adds to the complexity of the process. I cannot tell you if and when we will add formatting configurations. We have a large list of issues we have to fix or features we want to add. If you think a formatting configuration is really important for you, you might write us an email using the contacts listed above.

> Also, I found some possible contradiction in the coding conventions. Wouldn't the following recommendation trigger a ReadabilityElseAfterReturn warning:

Thanks for reporting this inconsistency, we will fix this.

[1]: https://en.wikipedia.org/wiki/Abstract_syntax_tree
[2]: https://en.wikipedia.org/wiki/Finite-state_machine

I see. Thank you for the explanation.

Something else (sorry for being picky): It seems that the formatter enforces that brackets immediately follow branch statements (if, elseif etc.) and loops (for, while) without any space. I am not sure that this improves readability. It seems that most of these statements also have a space character in front of the bracket in the official manual (yet, at least for the for-loop the examples are inconsistent). I would vote for introducing a space character here or are there any good reasons for not having one?

A few more comments:

- I find the manual somewhat confusing regarding what every ipt command is doing exactly, i.e., which checks are applied and which things are changed under which circumstances. For example, I was under the impression that 'ipt lint --fix' only fixes issues as defined by the linting rules, but instead all formatting of 'ipt format' is applied as well.

- I find it confusing that there are different commands for different task in some cases (format, check) and a flag in other cases (lint with and without --fix). Maybe the manual could be a bit clearer about what each command does: maybe have a table listing what is checked, what are the rules, is formatting / fixing applied, and is the file itself modified or just the result printed in the output. I personally would find it clearer if there was only 'format' and 'check' with, e.g., a '--strict' flag to indicate that the linting rules are observed as well. But, of course, this is just to give you and idea what might be difficult to follow for me.

- The manual is a bit lax with the usage of 'errors', which seems to include 'warnings' in some cases (e.g., for linting). As I understand it, if there are errors then the linter will only display these and no warnings. And 'ipt check' will of course display no linter warnings. Maybe the manual could be adjusted to make this distinction?

- The 'Special features' category seems to apply to 'ipt format' and 'ipt lint --fix', right? Wouldn't it be better to name it 'Formatting features' then?

- Finally, is there a way to output results for rules with severity 'none' (and possibly 'info' if there are new rules in the future)? Otherwise these rules are a not very useful if formatting is avoided.