Skip to content

flake: increase flake max line length #297

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

Merged
merged 1 commit into from
Aug 15, 2025
Merged

flake: increase flake max line length #297

merged 1 commit into from
Aug 15, 2025

Conversation

Managor
Copy link
Member

@Managor Managor commented Aug 15, 2025

No description provided.

@Managor Managor changed the title flake: increase flake line length flake: increase flake max line length Aug 15, 2025
Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

The original value was set a bit higher with respect to PEP8 which suggested a max of 80 lines to view code in terminals with ease. But nowadays it's fine to have 120 or higher so let's proceed with the changes.

@Managor
Copy link
Member Author

Managor commented Aug 15, 2025

Yeah traditional terminals are not being used anywhere so the original limit makes no sense.

@Managor Managor merged commit 7944209 into main Aug 15, 2025
63 checks passed
@Managor Managor deleted the flake-settings branch August 15, 2025 03:24
@kbdharun
Copy link
Member

kbdharun commented Aug 15, 2025

I think black defaults to 89 line length will check online and get back to see if we need to update/define a custom value in a config file.

@sebastiaanspeck
Copy link
Member

This is the flake8 configuration in tldr itself: https://github.com/tldr-pages/tldr/blob/main/.flake8

@kbdharun
Copy link
Member

Was going through related discussions online about max line length, some places suggest a maximum of 120 (since it's used by default in IDE's like PyCharm to match with IntelliJ) and any nested single line won't be too long.

Or else, as majority suggests online (and to be uniform with main repo), let's keep the current value of 88 as it is to not modify black's config, but instead use line breaks and backslashes to divide contents over multiple lines. And to prevent sending PR before addressing this, maybe we can add a precommit hook.

@Managor
Copy link
Member Author

Managor commented Aug 15, 2025

But forced multiline looks ugly as hell. See the warning message in the other PR. There's no reason for it to be split into 3-4 lines

@kbdharun
Copy link
Member

kbdharun commented Aug 15, 2025

But forced multiline looks ugly as hell. See the warning message in the other PR. There's no reason for it to be split into 3-4 lines

Agreed, the current tldr.py file is so long since we are using line breaks due to the 88 restriction, 150 should be fine for normal use in an IDE, but 100/120 would be fine if you want to open 2 files side by side.

For instance, this is with the current 88 configuration in my laptop in full screen, there is still a small side scroll in some places (so if it's 150, it will be too large):

image

If we agree on setting it as 100/120 here, let's update tldr's flake8 config to the same for consistency across places.

@Managor
Copy link
Member Author

Managor commented Aug 15, 2025

120 is better than 88. I don't care as long as the minimum length is made longer. My main goal is to just get the warning message and autocomplete fixed and pushed into a new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants