Max line length #105

mdczaplicki · 2022-05-23T09:56:30Z

Small update to have max-line-length as a argument.

Sorry, but didn't have time to make tests pass. Some magic happens below 😛

Closes #90

DanielNoord

Thank you very much for taking this on @mdczaplicki. Very much appreciated!

I left some comments, let me know if you have any questions!

.gitignore

DanielNoord · 2022-05-23T10:02:42Z

docs/usage.rst

@@ -17,6 +17,7 @@ Current usage of ``pydocstringformatter``:
                                [--capitalize-first-letter  --no-capitalize-first-letter]
                                [--final-period  --no-final-period]
                                [--quotes-type  --no-quotes-type]
+                                [--max-line-length MAX_LINE_LENGTH]


It seems the CI fails here. You could try:

cd docs make html

That should update the docs.

Did that. Unfortunately pipelines still complain

pydocstringformatter/configuration/arguments_manager.py

pydocstringformatter/formatting/base.py

requirements-test.txt

DanielNoord · 2022-05-23T10:04:49Z

tests/data/format/max_line_length/function_docstrings.args

@@ -0,0 +1 @@
+--max-line-length 50


I'm not sure this works. We might need:

Suggested change

--max-line-length 50

--max-line-length=50

Oh, that's why :D

If I remember correctly it just reads all .args files and splits on every space it encounters. Then it basically calls:
pydocstringformatter --max-line-length 50 test.py. So these files should mimic how you would normally call the program on CLI

DanielNoord · 2022-05-23T10:07:25Z

tests/data/format/max_line_length/function_docstrings.py

@@ -0,0 +1,13 @@
+def func():


Basically the test should have 3 cases:

One multi-line summary clearly above 50 characters. > Summary should be wrapped.

One multi-line summary with a description above 50 chars. > Summary should be wrapped, but description shouldn't as we don't support that.

One single-line docstring that is 49 chars without the final quotes. See LinesChunk.
If it is a single-line docstring we need to add the final quotes on the same line, so the line is not actually 49 but 52, so we should indeed wrap. That is what LinesChunck is testing.

OK. Updated them. But for some reason it seems that 50 is not picked up in this test.

I think this is because the option doesn't use type. You could add a breakpoint somewhere after line_length = self.config.max_line_length but I think line_length will now be "50" instead of 50. This can be fixed by using type=int in the add_argument call.

It actually says that all docstrings are properly formatted. So it doesn't run

DanielNoord · 2022-05-23T11:18:16Z

tests/data/format/linewrap_summary/max_line_length_50.args

@@ -0,0 +1,2 @@
+--linewrap-full-docstring


@mdczaplicki I have pushed some changes. Turns out I was being very dumb.

Since line wrapping is still experimental it is not enabled by default. So for the test to run, it also needs this flag set.

No problem. Thanks for the help here :D

No worries and thanks very much for doing the initial work. It's much easier as maintainer to build on a PR that's 90% done than starting from scratch.

Sorry for the spam you're getting right now. There is something wrong with one of our test runs but I can only test it on a PR opened by somebody that is not a contributor... I'll clean the commits in this PR up after I have fixed the issue and then merge this!

Tests pass locally. 🎉

BTW. I believe there is a problem with make html. I was running it, but pipelines complained.

Also:

❯ cd docs ❯ pip install -e . Obtaining file:///home/marek/PycharmProjects/pydocstringformatter/docs ERROR: file:///home/marek/PycharmProjects/pydocstringformatter/docs does not appear to be a Python project: neither 'setup.py' nor 'pyproject.toml' found. ❯ pip install -r requirements-doc.txt Obtaining file:///home/marek/PycharmProjects/pydocstringformatter/docs (from -r requirements-doc.txt (line 1)) ERROR: file:///home/marek/PycharmProjects/pydocstringformatter/docs (from -r requirements-doc.txt (line 1)) does not appear to be a Python project: neither 'setup.py' nor 'pyproject.toml' found.

However it works when I go

cd ..

and then

pip install -r docs/requirements-doc.txt

Yeah it should be:

pip install -e . cd docs make html

This is due to how readthedocs install dependencies from root. So we need to install them from root as well...

coveralls · 2022-05-23T11:19:33Z

Pull Request Test Coverage Report for Build 2371300890

2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 2371299252:	0.0%
Covered Lines:	471
Relevant Lines:	471

💛 - Coveralls

DanielNoord

One final spelling change.

I'm also using this PR to test some changes to the primer. Sorry for the merge commit spam 😅

pydocstringformatter/configuration/arguments_manager.py

docs/usage.rst

github-actions · 2022-05-23T12:36:27Z

According to the primer, this change has no effect on the checked open source code. 🤖🎉

DanielNoord · 2022-05-23T12:37:08Z

Thanks once again @mdczaplicki, also for prompting me to final fix our CI 🎉

DanielNoord requested changes May 23, 2022

View reviewed changes

DanielNoord added this to the 0.6.0 milestone May 23, 2022

DanielNoord added the enhancement New feature or request label May 23, 2022

DanielNoord mentioned this pull request May 23, 2022

Cut lines that exceed a number of caracter and reorganize docstrings #7

Open

DanielNoord reviewed May 23, 2022

View reviewed changes

DanielNoord approved these changes May 23, 2022

View reviewed changes

pydocstringformatter/configuration/arguments_manager.py Outdated Show resolved Hide resolved

docs/usage.rst Outdated Show resolved Hide resolved

Add --max-line-length option

b62e77f

DanielNoord force-pushed the max-line-length branch from 0b51a74 to b62e77f Compare May 23, 2022 12:19

DanielNoord added 2 commits May 23, 2022 14:24

Merge branch 'main' into max-line-length

5089fa8

Merge branch 'main' into max-line-length

c746a45

This comment has been minimized.

Sign in to view

Merge branch 'main' into max-line-length

9d83c8d

DanielNoord merged commit b824fee into DanielNoord:main May 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Max line length #105

Max line length #105

mdczaplicki commented May 23, 2022 •

edited by DanielNoord

Loading

DanielNoord left a comment

DanielNoord May 23, 2022

mdczaplicki May 23, 2022

DanielNoord May 23, 2022

mdczaplicki May 23, 2022

DanielNoord May 23, 2022

DanielNoord May 23, 2022

mdczaplicki May 23, 2022

DanielNoord May 23, 2022

mdczaplicki May 23, 2022

DanielNoord May 23, 2022

mdczaplicki May 23, 2022

DanielNoord May 23, 2022

mdczaplicki May 23, 2022

DanielNoord May 23, 2022

coveralls commented May 23, 2022 •

edited

Loading

DanielNoord left a comment

This comment has been minimized.

github-actions bot commented May 23, 2022

DanielNoord commented May 23, 2022

Max line length #105

Max line length #105

Conversation

mdczaplicki commented May 23, 2022 • edited by DanielNoord Loading

DanielNoord left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 23, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2371300890

💛 - Coveralls

DanielNoord left a comment

Choose a reason for hiding this comment

This comment has been minimized.

github-actions bot commented May 23, 2022

DanielNoord commented May 23, 2022

mdczaplicki commented May 23, 2022 •

edited by DanielNoord

Loading

coveralls commented May 23, 2022 •

edited

Loading