Skip to content
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

Boltex/issue3760 #3761

Merged
merged 13 commits into from Jan 13, 2024
Merged

Boltex/issue3760 #3761

merged 13 commits into from Jan 13, 2024

Conversation

boltex
Copy link
Contributor

@boltex boltex commented Jan 10, 2024

Fixes #3760.

  • Update leoBridge to remove hard-coded cheat for LeoInteg.
    The Server now hooks onto 'open2' event to finalize newly created commander that
    would normally be added to the window tabs.
  • Add missing 'open' hook events to openExternalFile.
    This matches the two other source of opened documents in leoApp.py.
  • Added UNL support to leoserver.py.

@boltex boltex requested a review from edreamleo January 10, 2024 06:39
@boltex boltex added this to the 6.7.7 milestone Jan 10, 2024
Copy link
Contributor Author

@boltex boltex left a comment

Choose a reason for hiding this comment

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

Big improvements on leoserver for UNL click support and other general improvements to the server! :)

@boltex boltex marked this pull request as draft January 10, 2024 06:43
@boltex
Copy link
Contributor Author

boltex commented Jan 10, 2024

I've made it a draft for now as I'd like to add one more small feature to the server still :)

Copy link
Member

@edreamleo edreamleo left a comment

Choose a reason for hiding this comment

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

Looks good.

PR #3762 changes various documentation files, especially LeoDocs.leo. I've converted leoInteg to LeoInteg there. Ditto for leoJS to LeoJS.

So please revert LeoDocs.leo and acknowledgements.html.txt.txt to devel. That will make it easier for me to merge my branch.

Along with UNL handling, (Thanks to the new server 'open2' event logic) 'goto script' can now find button origin in other files like myLeoSettings and leoSettings.
@boltex
Copy link
Contributor Author

boltex commented Jan 11, 2024

@edreamleo Even better: I'll wait until you merge #3762 into devel, then I'll pull devel into #3760, making sure any conflicts are resolved and dealt into #3760 by myself, (if any).

Then you will be able to merge it into devel without any conflicts for sure.

The trouble of providing a branch to be merged smoothly without any conflicts to resolve should rest on the person making the new feature branch.

You should not have to worry about annoying merge conflicts on top of the rest! 😆

@edreamleo
Copy link
Member

@boltex I've merged #3762 into devel, so have at it :-)

@edreamleo
Copy link
Member

@boltex boltex marked this pull request as ready for review January 12, 2024 02:46
@boltex
Copy link
Contributor Author

boltex commented Jan 12, 2024

About unit tests

The new leoserver system now uses the open2 'doHook' event.

When running tests, if I run only the 'TestLeoServer' tests, they pass. But if I launch all tests, I think something in the unit tests preceding the server tests turns off the 'doHook' events, and they fail.

I don't know enough about the python unit tests system to resolve this :/

(as you can see in the screenshot below, in vscode I can press 'play' on on any unit tests subsection, and pressing play to run only the server tests makes them run without issue, but running the whole suite of Leo's unit tests make the server tests fails because the hook events do not trigger somehow)
image

Below is that same section after trying to run all tests:
image

@boltex boltex marked this pull request as draft January 12, 2024 03:11
@boltex boltex marked this pull request as ready for review January 12, 2024 03:37
@boltex
Copy link
Contributor Author

boltex commented Jan 12, 2024

@tbpassin @edreamleo These modifications on leoserver.py enables many new features in leoInteg, among which, the possibility to right-click on an @button originating from another file (myLeoSettings.leo containing @buttons outlines) and choose 'goto-script' to find the @buttons script origin node! 😄 Thanks to @tbpassin for letting me know about this missing feature when he tested LeoJS! (which also now supports this feature)

I'll release the new LeoInteg version as soon as Leo 6.7.7. is launched! 🚀

@edreamleo edreamleo marked this pull request as draft January 12, 2024 15:16
@edreamleo
Copy link
Member

I just tested this branch. Here are the results:

ft.cmd
..........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................EException in test_most_public_server_methods: 'find_quick_changed' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'find_quick_history' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'find_quick_marked' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'find_quick_timeline' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'get_goto_panel' get_goto_panel: exception doing nav search: 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'get_search_settings' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'goto_nav_entry' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'interactive_search' 'NoneType' object has no attribute 'set_find_text'
Exception in test_most_public_server_methods: 'nav_clear' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'nav_headline_search' nav_headline_search: exception doing nav headline search: 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'nav_search' 'Commands' object has no attribute 'patched_quicksearch_controller'
Exception in test_most_public_server_methods: 'replace' replace: Running change operation gave exception: 'NoneType' object has no attribute 'get_settings'
Exception in test_most_public_server_methods: 'replace_all' replace_all: Running change operation gave exception: 'NoneType' object has no attribute 'get_settings'
Exception in test_most_public_server_methods: 'replace_then_find' replace_then_find: Running change operation gave exception: 'NoneType' object has no attribute 'get_settings'
.E..................................................................................................................................
======================================================================
ERROR: test_find_commands (leo.unittests.core.test_leoserver.TestLeoServer.test_find_commands)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 1937, in find_all
    settings = fc.ftm.get_settings()
               ^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'get_settings'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Repos\leo-editor\leo\unittests\core\test_leoserver.py", line 194, in test_find_commands
    answer = self._request(method, {"log": log, "find_text": "def"})
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\unittests\core\test_leoserver.py", line 64, in _request
    response = server._do_message(d)
               ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 4769, in _do_message
    result = func(action, param)
             ^^^^^^^^^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 4785, in _do_server_command
    return func(param)
           ^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 1940, in find_all
    raise ServerError(f"{tag}: exception running 'find all': {e}")
leo.core.leoserver.ServerError: find_all: exception running 'find all': 'NoneType' object has no attribute 'get_settings'

======================================================================
ERROR: test_open_and_close (leo.unittests.core.test_leoserver.TestLeoServer.test_open_and_close)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Repos\leo-editor\leo\unittests\core\test_leoserver.py", line 181, in test_open_and_close
    self._request(action, package)
  File "C:\Repos\leo-editor\leo\unittests\core\test_leoserver.py", line 64, in _request
    response = server._do_message(d)
               ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 4769, in _do_message
    result = func(action, param)
             ^^^^^^^^^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 4785, in _do_server_command
    return func(param)
           ^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 1336, in close_file
    c = self._check_c(param)
        ^^^^^^^^^^^^^^^^^^^^
  File "C:\Repos\leo-editor\leo\core\leoserver.py", line 4591, in _check_c
    raise ServerError(f"{tag}: no open commander")
leo.core.leoserver.ServerError: _check_c: no open commander

----------------------------------------------------------------------
Ran 895 tests in 6.709s

FAILED (errors=2)
ruff leo\core,  leo\commands, leo\plugins\qt...
leo\core\leoserver.py:1606:29: F841 [*] Local variable `e` is assigned to but never used
Found 1 error.
[*] 1 fixable with the `--fix` option.

mypy-leo
Success: no issues found in 260 source files
Done!

C:\Repos\leo-editor>

@edreamleo
Copy link
Member

@boltex I'll release Leo 6.7.7 as soon as we merge this PR. I'll move all other issues to 6.7.8.

@boltex
Copy link
Contributor Author

boltex commented Jan 13, 2024

@edreamleo Do you know why event hooks do not trigger when running all tests but they trigger when running individual tests like I posted above? That would greatly help me resolve this! (thanks in advance, in the meantime i'll investigate some more to see if I can find why - probably a switch that gets turned off by a previous test running in the chain)

@boltex
Copy link
Contributor Author

boltex commented Jan 13, 2024

@edreamleo I found a way! 😄

@boltex boltex marked this pull request as ready for review January 13, 2024 03:14
Copy link
Member

@edreamleo edreamleo left a comment

Choose a reason for hiding this comment

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

Elegant fix at line 1275. However, the test should use isinstance:

if g.unitTesting and isinstance(g.app.pluginsController, NullObject):

@edreamleo
Copy link
Member

@boltex Or probably isinstance(g.app.pluginsController, g.NullObject)

@edreamleo
Copy link
Member

@boltex dc919a5 fixes some quirps.

The most serious: pylint pointed out that at lines 5124 and 5128 the code should raise the indicated exceptions. However, running the unit tests raises the exception at line 5128 (marked with '###'). Perhaps if not g.unitTesting: should replace if 0: ?

What do you think?

@edreamleo
Copy link
Member

@boltex ad5d0f3 suppresses the exception during unit tests. I'll approve the present code. Feel free to tweak.

@boltex
Copy link
Contributor Author

boltex commented Jan 13, 2024

I've looked at what you've tuned up and it looks good. I've also re-tested the new features just to make sure. It all still works, so this looks very good to me 😄

@boltex boltex requested a review from edreamleo January 13, 2024 21:19
@boltex
Copy link
Contributor Author

boltex commented Jan 13, 2024

@edreamleo Many thanks to you for those fixes!

@edreamleo edreamleo merged commit 712d9ac into devel Jan 13, 2024
@edreamleo edreamleo deleted the boltex/issue3760 branch January 13, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify leoserver.py to support handling UNL links from connected clients.
2 participants