archlinux / archinstall

Arch Linux installer - guided, templates etc.
GNU General Public License v3.0
6.32k stars 541 forks source link

Reduce the max line length from 220 to 160 by wrapping some lines #2867

Closed correctmost closed 3 days ago

correctmost commented 4 days ago

PR Description:

This will make it easier to auto-format import sections.

svartkanin commented 4 days ago

@correctmost this might be somewhat controversial :) I'd rather leave this at the current value as I'm sure there's loads of opinions on this, unless there's a significant benefit on reducing it.

@Torxed any thoughts?

Torxed commented 4 days ago

Yeah, this will probably break down to personal preferences and nothing more. But I would like to keep the longer line length if possible.

correctmost commented 4 days ago

My main goal with this PR was to make it easier to auto-sort import sections:

git stash
git checkout master
git checkout -b sort-imports
ruff check --select=I --fix
git diff

With a limit of 220, some import lines are very long :).

Sorting imports is mostly a cosmetic change, but it can also help make the codebase more uniform and make it easier to see dependencies between modules.

Torxed commented 4 days ago

And there's no way to force multi-imports to use:

from x import (
    a,
    b
)

Without this?

svartkanin commented 4 days ago

Is 160 a number that will certainly fix it, or fix it for the current state and in the future it may have to be changed again as some imports require an even lower length to be set?

correctmost commented 4 days ago

And there's no way to force multi-imports to use:

from x import (
    a,
    b
)

Without this?

I tried Ruff's various sort settings and didn't see one that would yield that formatting, unfortunately.

Is 160 a number that will certainly fix it, or fix it for the current state and in the future it may have to be changed again as some imports require an even lower length to be set?

I picked 160 to limit the number of non-import lines that would need to be updated to accommodate the new maximum. It seems like a reasonable limit to stick with in the future, given some of the long lines in the codebase.

Here's the import diff between 220 (before) and 160 (after):

diff --git a/archinstall/__init__.py b/archinstall/__init__.py
index 2b400a19..97341488 100644
--- a/archinstall/__init__.py
+++ b/archinstall/__init__.py
@@ -13,7 +13,18 @@ from . import default_profiles
 from .lib import disk, exceptions, interactions, locale, luks, mirrors, models, networking, packages, profile
 from .lib.boot import Boot
 from .lib.configuration import ConfigurationOutput
-from .lib.general import JSON, UNSAFE_JSON, SysCommand, SysCommandWorker, clear_vt100_escape_codes, generate_password, json_stream_to_structure, locate_binary, run_custom_user_commands, secret
+from .lib.general import (
+   JSON,
+   UNSAFE_JSON,
+   SysCommand,
+   SysCommandWorker,
+   clear_vt100_escape_codes,
+   generate_password,
+   json_stream_to_structure,
+   locate_binary,
+   run_custom_user_commands,
+   secret,
+)
 from .lib.global_menu import GlobalMenu
 from .lib.hardware import GfxDriver, SysInfo
 from .lib.installer import Installer, accessibility_tools_in_use
diff --git a/archinstall/lib/disk/partitioning_menu.py b/archinstall/lib/disk/partitioning_menu.py
index ddcda62d..77e22bf4 100644
--- a/archinstall/lib/disk/partitioning_menu.py
+++ b/archinstall/lib/disk/partitioning_menu.py
@@ -11,7 +11,19 @@ from ..hardware import SysInfo
 from ..menu import ListManager
 from ..output import FormattedOutput
 from ..utils.util import prompt_dir
-from .device_model import BDevice, BtrfsMountOption, DeviceGeometry, FilesystemType, ModificationStatus, PartitionFlag, PartitionModification, PartitionType, SectorSize, Size, Unit
+from .device_model import (
+   BDevice,
+   BtrfsMountOption,
+   DeviceGeometry,
+   FilesystemType,
+   ModificationStatus,
+   PartitionFlag,
+   PartitionModification,
+   PartitionType,
+   SectorSize,
+   Size,
+   Unit,
+)
 from .subvolume_menu import SubvolumeMenu

 if TYPE_CHECKING:
diff --git a/archinstall/lib/interactions/__init__.py b/archinstall/lib/interactions/__init__.py
index 037329d1..bd269d8c 100644
--- a/archinstall/lib/interactions/__init__.py
+++ b/archinstall/lib/interactions/__init__.py
@@ -1,4 +1,11 @@
-from .disk_conf import get_default_partition_layout, select_devices, select_disk_config, select_main_filesystem_format, suggest_multi_disk_layout, suggest_single_disk_layout
+from .disk_conf import (
+   get_default_partition_layout,
+   select_devices,
+   select_disk_config,
+   select_main_filesystem_format,
+   suggest_multi_disk_layout,
+   suggest_single_disk_layout,
+)
 from .general_conf import (
    add_number_of_parallel_downloads,
    ask_additional_packages_to_install,
diff --git a/archinstall/lib/locale/__init__.py b/archinstall/lib/locale/__init__.py
index a22d50b4..57fb999b 100644
--- a/archinstall/lib/locale/__init__.py
+++ b/archinstall/lib/locale/__init__.py
@@ -1,2 +1,10 @@
 from .locale_menu import LocaleConfiguration
-from .utils import list_keyboard_languages, list_locales, list_timezones, list_x11_keyboard_languages, set_kb_layout, verify_keyboard_layout, verify_x11_keyboard_layout
+from .utils import (
+    list_keyboard_languages,
+    list_locales,
+    list_timezones,
+    list_x11_keyboard_languages,
+    set_kb_layout,
+    verify_keyboard_layout,
+    verify_x11_keyboard_layout,
+)
diff --git a/archinstall/tui/curses_menu.py b/archinstall/tui/curses_menu.py
index 62fd5c3f..e9b8b713 100644
--- a/archinstall/tui/curses_menu.py
+++ b/archinstall/tui/curses_menu.py
@@ -16,7 +16,22 @@ from typing import TYPE_CHECKING, Any, Literal
 from ..lib.output import debug
 from .help import Help
 from .menu_item import MenuItem, MenuItemGroup
-from .types import SCROLL_INTERVAL, STYLE, Alignment, Chars, FrameProperties, FrameStyle, MenuCell, MenuKeys, Orientation, PreviewStyle, Result, ResultType, ViewportEntry, _FrameDim
+from .types import (
+   SCROLL_INTERVAL,
+   STYLE,
+   Alignment,
+   Chars,
+   FrameProperties,
+   FrameStyle,
+   MenuCell,
+   MenuKeys,
+   Orientation,
+   PreviewStyle,
+   Result,
+   ResultType,
+   ViewportEntry,
+   _FrameDim,
+)

 if TYPE_CHECKING:
    _: Any
diff --git a/pyproject.toml b/pyproject.toml
index d62e8018..e696e076 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -163,7 +163,7 @@ additional-builtins = ["_"]
 [tool.ruff]
 target-version = "py311"
 builtins = ["_"]
-line-length = 220
+line-length = 160

 [tool.ruff.lint]
 select = [

And here's the longest import line that would be present with a limit of 160:

from parted import Device, Disk, DiskException, FileSystem, Geometry, IOException, Partition, PartitionException, freshDisk, getAllDevices, getDevice, newDisk
Torxed commented 4 days ago

Haha, that is one hell of an import statement. That diff looks great, lets go for it!