Reconsider NVDA code style, linting and auto formatting

[LeonarddeR]

With our switch to a newer Python coming closer and closer, I'd like to bring up a controversial subject again, namely the NVDA code style, linting and auto formatting.

Lately, Microsoft Visual Studio Code has become a major game changer in development for blind and visually impaired folks because it is a accessibility focused IDE with many features that make life of a developer a charm. To put it another way, the days of coding in NotePad++ lay or should lay far behind us. However, the developments in Code do not stand still, and recently they dropped support for the older version of Flake8 we're using. That brings us to the first problem, we are stukc to an older version of Flake8 because we rely on Flake8tabs and that Python extension is no longer maintained. It also brings me to the first controversial suggestion, switching from tabs to spaces for indentation. I have worked with both Visual Studio and VS Code for several years now, and apart from the case where space indentation takes more space on a braille display, I see no disadvantages, as a modern IDE can handle this very well.

Switching from tabs to spaces means that we can rely on the most recent version of Flake8, however when we consider changing this, I think we should also consider automatic code formatting with a reliable formatter like Black. To be honest, I think that the NVDA project puts too much code formatting burden on the shoulders of code contributors. Furthermore, after several years of enforcing a code formatting style for the NVDA source code, I still pretty often find myself in a situation where I have to edit code and reformat it in the process. Honestly, I believe that in modern times, not using automatic formatting of our Python and C++ code is a major disadvantage and cause of disappointment.

[seanbudd]

We'd like to hear more community feedback here.

If we end up dropping tabs for spaces, would that affect anyone?

If we end up switching over, we will be refactoring the entire code base.
Considering this, we should also do other entire codebase refactor tasks e.g. autoformatting, and switching line endings to LF.

[LeonarddeR]

As an update that is probably pretty relevant to us, the experimental formatter in Ruff now supports tab indentation. The formatter in Ruff is mostly based on Black. Black usually follows the code style NVDA uses, except for the extra level of indentation PEP8 and NVDA use for function parameters.

[lukaszgo1]

You brought up several thinks here, which are IMHO, more or less independent from each other. While I sort of understand that fixing all your gripes with NVDA's code formatting in one step is tempting, aiming for something that big may mean nothing gets done. In contrast I'd like to suggest moving in smaller steps, and reducing scope of this discussion:

• You have mentioned linting and reformatting of C++ code which this is totally independent from the main subject, can be safely done separately, and would be noticeable to a smaller pool of developers. Therefore it stands to reason not to discuss it here, rather in Add lint check for C++ changes #12407
• It is certainly true that being stuck on an old version of Flake8 is to say the least suboptimal. The fact that VS Code requires newer version is only one of the reasons, another being that we need to use a lot of NOQA comments for imports used only for type annotations (this is fixed in more recent version of Flake8). Switching would obviously require finding a solution to the flake8-tabs plugin, I'm not sure if anyone contacted its author as to their plans of updating it, or it has just been assumed that it is abandoned. The second problem with the more recent versions of Flake8 is the fact that their dropped ability To lint only a diff of changes (I assume that is why you draw a connection between updating, and being force to reformat the entire code base). It seems there are several tools which can help here, Darker, and lint-diffs. We can also upgrade to Flake8 V5 (that is the last version which supports linting of changed lines) when the problems with the tabs plugin is solved, and then investigate how to lint a diff. Note that I am not personally opposed either to auto formatting, nor to use of spaces in place of tabs (more on that below), but bringing up different options may help in moving in smaller increments
• When the Flake8 problem would be fixed we can subsequently focus on auto formatting, It looks like you have investigated several options, though deciding on tabs vs spaces should b done first. While Ruff formatter (and Ruff as a whole) looks really promising, it is the only formatter and linter which forces us to reformat the entire code base (for Black Darker mentioned above can format a diff, and for Yapf yapf-diff should do the trick). This gives us best of both words - contributors do not need to reformat code by hand any longer, and we avoid disruptive changes. Obviously we can probably use something more configurable than Black like cercis

As to the tabs vs spaces from the technical side I am personally indifferent, however it would be good to have opinions from people contributing regularly who may have different perspectives. In particular I'm curious what @CyrilleB79 thinks - I believe you are using some remaining vision, and when discussing tabs for accessibility their configurability for VI programmers is often mentioned. I would also be nice to know why tabs were used in the first place from either @jcsteh or @michaelDCurran - I've never found the Notepad++ argument much convincing, since with correct configuration it can also auto-indent similarly to VSCode / PyCharm.
It also looks like the change from tabs to spaces is mostly due to lack of adequate tooling which can handle tab based indentation for Python, which itself worries me. Changing a code style due to tools limitation is not what should happen. The counter argument may be that, since tabs are standard in the Python community, we can gain more contributors by following it, but I'm not sure to what extend this would hold true. NVDA is quite niche project, with a very small pool of semi regular contributors unfortunately, and I haven't hear from anyone who wanted to contribute but was disinclined by our coding style.

[LeonarddeR]

In contrast I'd like to suggest moving in smaller steps, and reducing scope of this discussion:

I certainly agree with that as long as the smaller steps don't make things more complex for people. When people are easily confused by changes, it is sometimes better to change everything at once.

* You have mentioned linting and reformatting of C++ code which  this is totally independent from the main subject, can be safely done separately, and would be noticeable to a smaller pool of developers. Therefore it stands to reason not to discuss it here, rather in [Add lint check for C++ changes #12407](https://github.com/nvaccess/nvda/issues/12407)

Agreed.

It looks like you have investigated several options, though deciding on tabs vs spaces should b done first. While Ruff formatter (and Ruff as a whole) looks really promising, it is the only formatter and linter which forces us to reformat the entire code base (for Black Darker mentioned above can format a diff, and for Yapf yapf-diff should do the trick). This gives us best of both words - contributors do not need to reformat code by hand any longer, and we avoid disruptive changes. Obviously we can probably use something more configurable than Black like cercis

There are several reasons why I would prefer switching to Ruff as a whole:

• Just one tool that does it all
• Automatic fixes for several violations
• Support for tabs in the formatter, so we can drop the tabs vs spaces discussion.

Note that Ruff kan not yet lint a diff, though Riff can do this to some extend. Also there is an issue for baseline support for Ruff

All in all, we don't need to rush this.

[ruifontes]

Hello! Às 18:03 de 10/10/2023, Łukasz Golonka escreveu: As to the tabs vs spaces from the technical side I am personally indifferent, however it would be good to have opinions from people contributing regularly who may have different perspectives. Regarding technical side, I do not know... But, for a Braille display user, having tabs instead of spaces save a lot of cells! I have already seen code with 4 spaces intead of a tab... In a 40 cells Braille display, 16 spaces instead of 4 tabs use almost half of the display... Rui Fontes

[seanbudd]

It seems that our current version of flake8 doesn't support modern python syntax such as the walrus operator :=.
build failure.
Now that we are on python 3.11 we should prioritise finding a new linting solution so we can make the most of modern python features.

[josephsl]

Hi,

In this case, I vote for Ruff (there is a PyPI package that will convert Flake8 config file to pyproject.toml for use by Ruff and other tools) unless Flake8-tabs can be maintained somehow (likely not). The fact that Flake8 3.9.2 does not support syntax introduced in Python 3.8 and later blocks people frof submitting/implementing things such as assignment expressions and structured pattern matching (these two can modernize conditional statements and loops scattered throughout NVDA code base and add-ons).

Thanks.

[lukaszgo1]

In contrast I'd like to suggest moving in smaller steps, and reducing scope of this discussion:

I certainly agree with that as long as the smaller steps don't make things more complex for people. When people are easily confused by changes, it is sometimes better to change everything at once.

For most contributors linting is an automatic process - the developer just executes a script, and the list of eventual violations is shown, so I'm not sure what, aside from the change of indentation standard, can be confusing here.

* Just one tool that does it all

I was never, regardless o what is discussed, convinced by arguments like these. As mentioned above we automate linting for contributors, (I assume we would aim to automate reformatting as well), so what happens under the hood is not that important.

Note that Ruff kan not yet lint a diff, though Riff can do this to some extend. Also there is an issue for baseline support for Ruff

Thanks - I've looked for a similar solution before commenting here, but didn't manage to find anything useful.

All in all, we don't need to rush this.

Given the latest issues with our old version of Flake8 not supporting the walrus operator, it seems circumstances has changed slightly.

[lukaszgo1]

To move this forward I have contacted author of flake8-tabs, to establish if this plugin is going to be maintained in the future, as that may influence our decisions.
After some reading about Ruff, I am not sure we can just use it as a drop-in replacement for flake8 in its current state. As far as I understand, while it indeed supports indenting with tabs this is implemented only in the formatter. In other words linting a diff would not flag any indentation related violations.
As a stop-gap solution we can also upgrade the version of Flake8 we're using to 4.0.1. It supports both the walrus operator and flake8-tabs. Obviously such interim measures are not the best, but they would buy us time to research, without limiting us to the Python 3.7 syntax.

[lukaszgo1]

Assuming the mass linting would not be extremely time consuming (many errors flagged by Flake8 currently cannot be fixed with auto formatting) that is one possible approach indeed.

[LeonarddeR]

Note that Ruff can perform automatic fixes for several rules.
It might be interesting to try a proof of concept here:

1. Format/autolint the whole code base with Ruff
2. Find out how many unfixable lint errors remain.

However, I'm not sure how git blame behaves when we ignore the commit(s) that autoformat/lint. Will it still be able to track down the original author from the history when it causes a massive rewrite?

[lukaszgo1]

Note that Ruff can perform automatic fixes for several rules. It might be interesting to try a proof of concept here:

1. Format/autolint the whole code base with Ruff

2. Find out how many unfixable lint errors remain.

When doing this we should be pretty careful to enable only fixes marked as safe, and preferably look at the full list of auto-fixes before starting, so that more questionable ones can be disabled in the config.

However, I'm not sure how git blame behaves when we ignore the commit(s) that autoformat/lint. Will it still be able to track down the original author from the history when it causes a massive rewrite?

I'm not sure either - that is probably something we would need to find out experimentally.

[lukaszgo1]

To move this forward I have contacted author of flake8-tabs, to establish if this plugin is going to be maintained in the future, as that may influence our decisions.

According to the Flake8-tabs developer while they do not intent to work on it very actively, they are more than willing to accept patches, and make new Pypi release afterwards.

[LeonarddeR]

I personally wouldn't like to rely on something that is unmaintained.

[kds215]

I'm trying to help a blind user (experienced C++ VStudio user) read python.py files. Python's lack of a clearly defined end statement such as ";" or "{...}" (like in C++ or perl) makes it very difficult reading python code with a JAWS reader.
An automatic simple closing comment like "#end class " or "#end def " would go a long way helping a blind user consuming python files.

[XLTechie]

@kds215 I'm not actually sure if you're asking for advice, or making a proposal for the NVDA codebase. If you're asking for advice, here is mine.

What you describe is the oldest problem faced by blind Python coders.
The solutions I know are:

1. Use a feature which indicates indentation. NVDA provides a tones based indentation marker that works well for this if coding in NotepadPlusPlus or any text editor.
   You can also use normal "X spaces Y tabs" indentation notices, though personally my brain works better with beeps.
   Also, hopefully the access software is smart enough to only tell you about indentation when it changes, so you only have to remember at what indent level your class, method, etc. starts. Since most classes and methods and such will share the same indent level as others of the same kind, this becomes second nature.
2. Use a 40/80 cell braille display to give you a "feel" for indentation.
3. Use an editor that provides indent based block collapse/expand. Notepad++ does this.
   In that case, collapse the blocks you aren't working on, and expand the ones you are.
4. Use an IDE which provides some sort of segregation/containerization. I have not used Visual Studio/VS Code, but I'm sure there is a workable mechanism therein as a lot of blind coders do use those IDEs.
5. While learning and getting used to things, there is technically nothing wrong with putting braces in comments, as long as you also remember to indent appropriately. For example:

if myVariable is None:  #{
    doSomething()
    doSomethingElse()  #}
else:  #{
    andNow.forSomethingCompletelyDifferent()  #}

(or whatever placement seems wise to the user)
Of course, no production code should look like that, and the user will have to get over it quickly, but it may help while learning.

[kds215]

Hi Luke, thanks for your response - I really did not expect an answer. My blind friend & I are coders from the old days of C/C++ and Perl. We’re comfortable working with visible markers defining start/end of code blocks. I’m in the process of implementing something similar along the lines you suggested [ #{ … #} ] to give him better python guidance while navigating blank line sounds coming from his JAWS reader. Thanks. -Klaus. P.S. He will be my beta tester for sure.

…

On Oct 27, 2023, at 2:16 AM, Luke Davis ***@***.***> wrote: @kds215 <https://github.com/kds215> While this is off topic for this discussion, I will give you at least one response. What you describe is the oldest problem faced by blind Python coders. The solutions I know are: Use a feature which indicates indentation. NVDA provides a tones based indentation marker that works well for this if coding in NotepadPlusPlus or any text editor. You can also use normal "X spaces Y tabs" indentation notices, though personally my brain works better with beeps. Also, hopefully the access software is smart enough to only tell you about indentation when it changes, so you only have to remember at what indent level your class, method, etc. starts. Since most classes and methods and such will share the same indent level as others of the same kind, this becomes second nature. Use a 40/80 cell braille display to give you a "feel" for indentation. Use an editor that provides indent based block collapse/expand. Notepad++ does this. In that case, collapse the blocks you aren't working on, and expand the ones you are. Use an IDE which provides some sort of segregation/containerization. I have not used Visual Studio/VS Code, but I'm sure there is a workable mechanism therein as a lot of blind coders do use those IDEs. While learning and getting used to things, there is technically nothing wrong with putting braces in comments, as long as you also remember to indent appropriately. For example: if myVariable is None: #{ doSomething() doSomethingElse() #} else: #{ andNow.forSomethingCompletelyDifferent() #} (or whatever placement seems wise to the user) Of course, no production code should look like that, and the user will have to get over it quickly, but it may help while learning. — Reply to this email directly, view it on GitHub <#15299 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHB65GNH2OOM73QYSXIE7PLYBNGSPAVCNFSM6AAAAAA3QXYWVOVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM3TGOJZHE4TA>. You are receiving this because you were mentioned.

[LeonarddeR]

I created astral-sh/ruff#8360 as the only remaining show stopper in the Ruff autoformatter for us.