Add command line option "--noreorder" to "add" command

[fwuehr1995]

In order to have the possibility to run the "doorstop add" command multiple times (e.g. in a shell script) without having to reorder the entire file each time the "add" command is invoked, it would be great to have the optional parameter to disable reordering.

[fwuehr1995]

Hi @neerdoc,

thanks for your comments. My main intention with this PR was to keep up the binary interface for everybody else who is not concerned of this and wants to stick to the previous approach with automatic reordering enabled. I was assuming that my PR is not interfering with this at all since you are not obligated to set the argument.

Nevertheless, if it would make sense to get rid of the reordering by default, I would agree that this PR is unnecessary. But this is of course a decision, that you guys as maintainers have to take. I would say yes, but I believe its not up to me.

Besides that, I am not sure if I get your concerns regarding the implementation in document.py right, I tested my implementation and it fully works. By not specifying the argument at the "doorstop add" call, the reordering parameter of the add_item function stays at the default value which is True. If the argument is specified though, this changes to false due to the assignment of "reorder=args.noreorder" in the actual function call. I tested it multiple times and it seems to work, so I decided it was good to go.

Regarding missing tests, I am happy to provide them, but if we decided to remove the automatic reordering anyway my PR would be obsolete.

So the main question for me: How to proceed with your suggestion of removing the automatic reordering in general ?

[neerdoc]

Ah, you are correct! I did not notice that all args are added automatically to the "add_item" call!

@jacebrowning What do you think? Should "reorder" really be the default when adding items to a document? IMHO I believe that it introduces an unnecessary overhead and that the reordering should only be done when requested by the user.

[jacebrowning]

@neerdoc Can you clarify what you mean by "extra overhead"? Running the doorstop add command several times in a row seems relatively quick to me. Maybe this topic should be opened as an issue first for discussion.

[fwuehr1995]

@jacebrowning I am using the doorstop add command in a shell script for multiple times in a row to add to the same file, but I notice that the execution time of doorstop add is significantly increasing the bigger the file to add to gets. Without the reordering every time we can decrease the execution time for each "doorstop add" to about 60 percent of what it was before.

So my intention is basically to do the reordering once after all necessary items have been added and not everytime that something gets added.

This just for your information to classify the idea.

[neerdoc]

I would assume that the cost of the reordering is at least $O(n \space log \space n)$. This will be fine for small documents, but cost significantly for large documents.

[jacebrowning]

My assumption is that the scenario of adding lots of items at once is far less common than adding a single item. If that's true, then I think we should keep the default behavior but add this option to allow the default behavior to be skipped.

[neerdoc]

Alright, let's go with that then!

@fwuehr1995 if you can add tests for this functionality it should be an easy approve! (And update to the latest develop branch to get recent updates first.)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (dd68d8a) 96.25% compared to head (6c4749b) 96.25%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #625 +/- ## ======================================== Coverage 96.25% 96.25% ======================================== Files 41 41 Lines 5159 5160 +1 Branches 1214 1214 ======================================== + Hits 4966 4967 +1 Misses 121 121 Partials 72 72 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[fwuehr1995]

@neerdoc thanks for the approval, much appreciated. I saw that some checks were still failing. I ran them locally on my machine (Fedora 39, Python 3.12.1), all tests seem to run and pass, but if I run "make check" I see multiple issues that have nothing to do with my change as well. If I don´t modify the test "test_add_custom_server" as I did, it will fail in the test step with an assertion error (which is perfectly legitimate, since I change the function call). But I still don´t understand whats wrong with what I did and why the checks are not passing.

Here is the output of my "make check" run:

wuehr@fwuehr-thinkpadt14gen4:~/Projects/git/doorstop$ make check poetry run isort doorstop Skipped 1 files poetry run black doorstop

poetry run mypy doorstop --config-file=.mypy.ini Success: no issues found in 89 source files poetry run pylint doorstop --rcfile=.pylint.ini Exception on node <ImportFrom l.7 at 0x7f266ffaed50> in file '/home/fwuehr/Projects/git/doorstop/doorstop/cli/commands.py' Traceback (most recent call last): File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/pylint/checkers/imports.py", line 798, in _get_imported_module return importnode.do_import_module(modname) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/nodes/_base_nodes.py", line 146, in do_import_module return mymodule.import_module( ^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/nodes/scoped_nodes/scoped_nodes.py", line 527, in import_module return AstroidManager().ast_from_module_name( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/manager.py", line 232, in ast_from_module_name return self.ast_from_file(found_spec.location, modname, fallback=False) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/manager.py", line 124, in ast_from_file return AstroidBuilder(self).file_build(filepath, modname) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/builder.py", line 144, in file_build module, builder = self._data_build(data, modname, path) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/builder.py", line 204, in _data_build module = builder.visit_module(node, modname, node_file, package) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/rebuilder.py", line 254, in visit_module [self.visit(child, newnode) for child in node.body], ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/astroid/rebuilder.py", line 603, in visit visit_method = getattr(self, visit_name) ^^^^^^^^^^^^^^^^^^^^^^^^^ AttributeError: 'TreeRebuilder' object has no attribute 'visit_typealias'

The above exception was the direct cause of the following exception:

Traceback (most recent call last): File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/pylint/utils/ast_walker.py", line 90, in walk callback(astroid) File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/pylint/checkers/imports.py", line 502, in visit_importfrom imported_module = self._get_imported_module(node, basename) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/fwuehr/Projects/git/doorstop/.venv/lib/python3.12/site-packages/pylint/checkers/imports.py", line 823, in _get_imported_module raise astroid.AstroidError from e astroid.exceptions.AstroidError Module doorstop.cli.commands doorstop/cli/commands.py:1:0: F0002: doorstop/cli/commands.py: Fatal error while checking 'doorstop/cli/commands.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-42.txt'. (astroid-error) Module doorstop.core.base doorstop/core/base.py:1:0: F0002: doorstop/core/base.py: Fatal error while checking 'doorstop/core/base.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-43.txt'. (astroid-error) Module doorstop.core.builder doorstop/core/builder.py:1:0: F0002: doorstop/core/builder.py: Fatal error while checking 'doorstop/core/builder.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-43.txt'. (astroid-error) Module doorstop.core.exporter doorstop/core/exporter.py:1:0: F0002: doorstop/core/exporter.py: Fatal error while checking 'doorstop/core/exporter.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-43.txt'. (astroid-error) Module doorstop.core.importer doorstop/core/importer.py:1:0: F0002: doorstop/core/importer.py: Fatal error while checking 'doorstop/core/importer.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-43.txt'. (astroid-error) Module doorstop.core.item doorstop/core/item.py:1:0: F0002: doorstop/core/item.py: Fatal error while checking 'doorstop/core/item.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-43.txt'. (astroid-error) Module doorstop.core.types doorstop/core/types.py:1:0: F0002: doorstop/core/types.py: Fatal error while checking 'doorstop/core/types.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-44.txt'. (astroid-error) Module doorstop.core.tree doorstop/core/tree.py:1:0: F0002: doorstop/core/tree.py: Fatal error while checking 'doorstop/core/tree.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-44.txt'. (astroid-error) Module doorstop.core.document doorstop/core/document.py:1:0: F0002: doorstop/core/document.py: Fatal error while checking 'doorstop/core/document.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-44.txt'. (astroid-error) Module doorstop.core.publishers.base doorstop/core/publishers/base.py:1:0: F0002: doorstop/core/publishers/base.py: Fatal error while checking 'doorstop/core/publishers/base.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-44.txt'. (astroid-error) Module doorstop.core.publishers.latex doorstop/core/publishers/latex.py:1:0: F0002: doorstop/core/publishers/latex.py: Fatal error while checking 'doorstop/core/publishers/latex.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-45.txt'. (astroid-error) Module doorstop.core.tests.test_yaml_validator doorstop/core/tests/test_yaml_validator.py:1:0: F0002: doorstop/core/tests/test_yaml_validator.py: Fatal error while checking 'doorstop/core/tests/test_yaml_validator.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-46.txt'. (astroid-error) Module doorstop.core.tests.init doorstop/core/tests/init.py:1:0: F0002: doorstop/core/tests/init.py: Fatal error while checking 'doorstop/core/tests/init.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-46.txt'. (astroid-error) Module doorstop.core.tests.helpers doorstop/core/tests/helpers.py:1:0: F0002: doorstop/core/tests/helpers.py: Fatal error while checking 'doorstop/core/tests/helpers.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-46.txt'. (astroid-error) Module doorstop.core.vcs.base doorstop/core/vcs/base.py:1:0: F0002: doorstop/core/vcs/base.py: Fatal error while checking 'doorstop/core/vcs/base.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-48.txt'. (astroid-error) Module doorstop.server.main doorstop/server/main.py:1:0: F0002: doorstop/server/main.py: Fatal error while checking 'doorstop/server/main.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in '/home/fwuehr/.cache/pylint/pylint-crash-2024-02-13-08-49-49.txt'. (astroid-error) poetry run pydocstyle doorstop scent.py

[fwuehr1995]

Anybody out there that can tell me what is going wrong with the checks ?

[neerdoc]

@fwuehr1995 I've had a look now and there are two different issues.

1. Windows tests are failing due to a problem with releasing file handles. I've updated the tests to handle this in PR #629, so once you rebase against it they should pass.
2. The linux tests fail due to make check detects changes, i.e., you have not managed to run make check on your code before pushing. Why this fails for you as you wrote in a comment, I cannot say. I would try with at clean install as see if that helps.

[fwuehr1995]

@neerdoc got it, there was some formatting missing. I pushed the changes generated by "make check", I hope we should be good to go now

[neerdoc]

You still have a problem with the Windows file release issue. Please rebase to the latest develop and it should pass.