[Feature][Python Console] Add Reformat code action

[YoannQDQ]

Description

Add a format code action in the Python Console Editor:

Settings

• Format on save: if enabled, formatting is applied just before saving the script
• Sort imports: Sort import statements using isort
• Max line length: Control how the formatter will wrap the lines, and controls the editor ruler
• Formatter: autopep8 or black
  • Level (autopep8 only) See Autopep8 aggressiveness level
  • Normalize quotes (black only): Replace all single quotes with double quotes if possible

black	autopep8

Sorting example

isort will sort import statement in three different groups:

• standard library imports (re, os, sys, json, ...)
• third-party modules (PyQt5, pandas, dateutil, ...)
• first-party modules (qgis, processing, ...)

[nyalldawson]

Great addition @YoannQDQ ! Since it's a new feature it'll need to wait till after 3.30 is released before it's eligible for merge.

[Gustry]

Nice addition indeed !

Are we sure that autopep8 is installed, even on windows from the binaries ?

[YoannQDQ]

Nice addition indeed !

Are we sure that autopep8 is installed, even on windows from the binaries ?

Hum... I'm not sure. Besides, I just added black as well, and I know for sure it is NOT installed (neither on Linux nor on Windows...).

Do you happen to know how we can ensure black and autopep8 are installed?

[Gustry]

Yes, for black it's sure it's not by default.
You can for instance :

try:
    import autopep8
    AUTO_PEP8_INSTALLED=True
except ImportError:
    AUTO_PEP8_INSTALLED=False

And then enable buttons carefully according to this variable ?

[YoannQDQ]

Yes, for black it's sure it's not by default. You can for instance :

try:
    import autopep8
    AUTO_PEP8_INSTALLED=True
except ImportError:
    AUTO_PEP8_INSTALLED=False

And then enable buttons carefully according to this variable ?

Thanks, but I meant "How to force black and autopep8 to be packaged with QGIS?".
If it is not desirable to install them by default, I could indeed check whether they're installed or not to enable/disable the feature accordingly, with a side note explaining how the formatters can be installed. (@nyalldawson any thoughts?)

[uclaros]

Nice!
I believe Re-Format code would be better wording.
Are the applied changes undo-able?

[YoannQDQ]

Nice! I believe Re-Format code would be better wording. Are the applied changes undo-able?

You're probably right. I'll change it. And yes, they are.

[3nids]

Once #51783 is merged, you'll be able to use the new structure for the settings in Python.

1. in QgsPythonUtils
   a. Create a tree node: static inline QgsSettingsTreeNode *sTreeConsole = QgsSettingsTree::treeRoot()->createChildNode( QStringLiteral( "console" ) );
   b. Declare the settings using QgsSettingsEntryString or any other implementation of QgsSettingsEntryBase
2. From Python, you can access the settings using QgsSettingsTree.node("console").childSetting("my_setting")

[nicogodet]

IMO, it definitely needs a try/except on import as proposed in #51733 (comment)

Une erreur est survenue lors de l'exécution du code suivant:
import console


Traceback (most recent call last):
  File "", line 1, in 
  File "D:\Downloads\QGIS-Portable\lib\python3.11\site-packages\qgis\utils.py", line 888, in _import
    mod = _builtin_import(name, globals, locals, fromlist, level)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\Downloads/QGIS-Portable/share/qgis/python\console\__init__.py", line 24, in 
    from .console import show_console  # NOQA
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\Downloads\QGIS-Portable\lib\python3.11\site-packages\qgis\utils.py", line 888, in _import
    mod = _builtin_import(name, globals, locals, fromlist, level)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\Downloads/QGIS-Portable/share/qgis/python\console\console.py", line 33, in 
    from .console_editor import EditorTabWidget
  File "D:\Downloads\QGIS-Portable\lib\python3.11\site-packages\qgis\utils.py", line 888, in _import
    mod = _builtin_import(name, globals, locals, fromlist, level)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\Downloads/QGIS-Portable/share/qgis/python\console\console_editor.py", line 33, in 
    import autopep8
  File "D:\Downloads\QGIS-Portable\lib\python3.11\site-packages\qgis\utils.py", line 888, in _import
    mod = _builtin_import(name, globals, locals, fromlist, level)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'autopep8'

Python error on startup

[YoannQDQ]

IMO, it definitely needs a try/except on import as proposed in #51733 (comment)

Yes my bad, I should have explicitly converted this PR to a draft before. I'm well aware that it does not work as it is right now, but I'm reluctant to just wrap the imports in try...except and call it a day. This feature will almost never be used if one has to manually install black, autopep8 and isort (Most QGIS users are not familiar with how to do it).

So, in my earlier comment, I asked how to properly ensure the relevant python modules are installed along QGIS, for every supported platform (at least Windows, MacOS, Ubuntu, Debian).
With that said, I may still have to resort to use try...except block.

Another possibility would be to use the following function to install modules as needed.

import importlib
import subprocess

def install(module):
    try:
        return importlib.import_module(module)
    except ModuleNotFoundError:
        pass
    try:
        import pip
    except ModuleNotFoundError:
        print(f"Pip not found. Try installing python3-pip with your package manager")
        return None
    
    result = subprocess.run(f"python -m pip install {module}", shell=True, capture_output=True)
    if result.returncode != 0:
        print(f"Pip error: {result.stderr.decode('utf8')}")
        return None

    try:
        return importlib.import_module(module)
    except ModuleNotFoundError:
        print("Module not found")
        return None

When the user triggers the reformat action, he would be prompted to confirm the installation of the required python modules (which could fail depending on the platform, and in offline situations).

def check_install(required_modules):
    missing = set()
    for module in set(required_modules):
        try:
            importlib.import_module(module)
        except ModuleNotFoundError:
            missing.add(module)
            
    if not missing:
        return True

    modules = "\n - ".join(sorted(missing))
    res = QMessageBox.question(
        iface.mainWindow(), 
        "Info", 
        "Some required python modules are not installed:\n - {0}\n Do you want to try and install them now?".format(modules)
    )
    if res != QMessageBox.Yes:
        return False
        
    for module in sorted():
        if install(missing):
            missing.remove(module)

    if not missing:
        QMessageBox.information(
            iface.mainWindow(), 
            "Info", 
            "Modules were successfuly installed!"
        )
        return True
        
    QMessageBox.warning(
        iface.mainWindow(), 
        "Warning", 
        "Some modules could not be installed"
    )
    return False

[kannes]

This feature will almost never be used if one has to manually install black, autopep8 and isort (Most QGIS users are not familiar with how to do it).

I wouldn't worry about that. This feature is quite advanced even for many people who write Python scripts in QGIS. I would bet that those who care about code formatting are very likely to also be capable of installing packages to their QGIS environment.

PS: This is a super cool feature, thank you! I'll use it a lot :)

[Gustry]

+1 with @kannes

I guess it also depends how visible are these buttons if the underneath library is not installed.

• Is the button only disabled with a nice tooltip explaining why ?
• or is the button enabled but the popup is coming on click ?

I guess the second one is better to "invite" people installing them. But from a UX point of view, the first one is better. I would go for 2 I think.

[nyalldawson]

@YoannQDQ would you mind splitting out the package installation parts of this PR for submission as a follow up pr? Those are slightly controversial and I'd like to merge just the actual reformatting part of this work before we tackle the harder questions...

[YoannQDQ]

@YoannQDQ would you mind splitting out the package installation parts of this PR for submission as a follow up pr? Those are slightly controversial and I'd like to merge just the actual reformatting part of this work before we tackle the harder questions...

Done.

[nyalldawson]

Thanks @YoannQDQ !

[github-actions]

@YoannQDQ
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

[github-actions]

@YoannQDQ
A documentation ticket has been opened at qgis/QGIS-Documentation#8105
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

[3nids]

This PR introduced settings without correctly declaring them as stated here #51733 (comment)