[jarun#704] fixed Windows support #705

LeXofLeviafan · 2023-12-05T20:33:18Z

fixes #704:

fixing error caused by I/O stream manipulation in browse(url) on Windows 11

also:

fixing color support on Windows 7-10
fixing tests to work on Windows (full tox run now succeeds on Windows 7-11… though I'm not actually sure how exactly this works on Windows 7 considering that it's no longer supported by CPython 3.9+ 😅)
fixing unregistered Pytest marker warning when running tests in Tox
fixing deprecation warning for text argument of BeatifulSoup .find*() methods

LeXofLeviafan · 2023-12-05T20:38:04Z

buku

@@ -4721,6 +4721,8 @@ def browse(url):
            # Found a GUI browser, suppress browser output
            browse.suppress_browser_output = True
            break
+    if sys.platform == 'win32':  # GUI apps have no terminal IO on Windows
+        browse.suppress_browser_output = False


Terminal I/O implementation on Windows forces the app to spawn a terminal window for it to actually make any I/O at all; due to this, I/O streams are disabled in pretty much all Windows applications with GUI.

…And manipulating I/O streams somehow breaks console process handle on Windows 11 😅

Also, in theory it should be possible to avoid I/O streams manipulation by setting browser.redirect_output & browser.background to True, but a quick practical test showed me that it's ridiculously easy to get a browser instance where this simply doesn't work :-/

LeXofLeviafan · 2023-12-05T20:40:09Z

buku

    except ImportError:
-        import pyreadline3 as readline  # type: ignore


pyreadline3 (…and incidentally pyreadline as well) actually exports a readline module, so this fallback export is both redundant and useless.

LeXofLeviafan · 2023-12-05T20:43:21Z

buku

    try:
-        import readline
+        import colorama
+        colorama.just_fix_windows_console()  # noop on non-Windows systems


ANSI colour codes are disabled by default on Windows 7-10, but are fairly easy to turn on.

This does not affect non-Windows systems, by design.

LeXofLeviafan · 2023-12-05T20:46:34Z

buku

@@ -3570,7 +3570,7 @@ def import_html(html_soup: BeautifulSoup, add_parent_folder_as_tag: bool, newtag
        comment_tag = tag.findNextSibling('dd')

        if comment_tag:
-            desc = comment_tag.find(text=True, recursive=False)
+            desc = comment_tag.find(string=True, recursive=False)


text parameter was renamed to string in BeautifulSoup 4.4.0 due to confusing semantics.

The deprecation warning was added in 4.11.0 to prevent usage of text as a keyword parameter.

LeXofLeviafan · 2023-12-05T20:48:33Z

tox.ini

+[pytest]
+markers =
+  non_tox: not run on tox
+


pytest.ini doesn't seem to be used when running tests via Tox. This appears to fix the “non-registered marker” warning.

LeXofLeviafan · 2023-12-05T20:51:33Z

tox.ini

@@ -56,13 +60,13 @@ commands =
    pytest --cov buku -vv -m "not non_tox" {posargs}

 [testenv:pylint]
-basepython = py311
+basepython = {env:BASEPYTHON:py311}


Windows doesn't seem to support Python detection correctly (likely due to not having a fixed file structure).

This allows to override configured Python version with an env variable.

LeXofLeviafan · 2023-12-05T20:54:26Z

tox.bat

@@ -0,0 +1,2 @@
+@if not defined BASEPYTHON set BASEPYTHON=python
+@tox.exe %*


On Windows only, running tox (with or without arguments) will default to this script, which ensures that BASEPYTHON is set (using the python executable of the venv, or the one in PATH if not using venvs for some reason)

LeXofLeviafan · 2023-12-05T20:57:10Z

tests/test_bukuDb.py

+    for bdb in bdbs:
+        try:
+            bdb.cur.close()
+            bdb.conn.close()


Most of the tests in this file fail on Windows due to DB connection not being closed between tests (since it causes DB file deletion to fail).

LeXofLeviafan · 2023-12-05T20:58:39Z

tests/test_bukuDb.py

 @pytest.fixture()
-def setup():
+def bukuDb():


Changing this fixture to a ‘managed’ BukuDB spawner, that ensures all connections are closed at the end of the test.

LeXofLeviafan · 2023-12-05T20:59:34Z

tests/test_bukuDb.py

+    def _bukuDb(*args, **kwargs):
+        nonlocal bdbs
+        bdbs += [BukuDb(*args, **kwargs)]
+        return bdbs[-1]


Alternatively this could be implemented as a class with BukuDb() method, but this way seems less clunky.

LeXofLeviafan · 2023-12-05T21:00:06Z

tests/test_bukuDb.py

+        return bdbs[-1]
+
+    yield _bukuDb
+    rmdb(*bdbs)


This is run after the test is completed

LeXofLeviafan · 2023-12-05T21:02:08Z

tests/test_bukuDb.py

-        )
+        home = os.path.expanduser("~")
+        dbdir_local_expected = (os.path.join(home, ".local", "share", "buku") if sys.platform != 'win32' else
+                                os.path.join(home, "AppData", "Roaming", "buku"))


On Windows, the default folder for application data is C:\%USERPROFILE%\AppData\Roaming\

LeXofLeviafan · 2023-12-05T21:03:39Z

tests/test_bukuDb.py

@@ -124,7 +144,7 @@ def test_get_default_dbdir(self):
        # -- home is defined differently on various platforms.
        # -- keep a copy and set it back once done
        originals = {}
-        for env_var in ["HOME", "HOMEPATH", "HOMEDIR"]:
+        for env_var in ["HOME", "HOMEPATH", "HOMEDIR", "APPDATA"]:


%APPDATA% is used on Windows to locate application data folder

LeXofLeviafan · 2023-12-05T21:04:38Z

tests/test_bukuDb.py

+        bdb.add_rec(**kwargs)
+        assert bdb.cur.execute.call_args[0][1] == exp_arg
+    finally:
+        bdb.cur = _cur


This needs to be restored for correct cleanup

LeXofLeviafan · 2023-12-05T21:06:10Z

tests/test_bukuDb.py

-        with open(exp_file.strpath, encoding="utf8", errors="surrogateescape") as f:
-            assert f.read()
+    f1 = NamedTemporaryFile(delete=False)
+    f1.close()


Similarly, temporary files need to be closed to be freely usable on Windows (i.e. to avoid access errors).

LeXofLeviafan · 2023-12-05T21:24:11Z

…Incidentally, spawning a new thread for each tab doesn't actually seem to do anything in Windows (tried disabling it, and there were effectively no changes on any Windows version, including Windows 7… and changing Python version didn't affect much either).

And according to the docs, waiting for the user to finish working with the browser is only implemented in Unix for “non-remote” browsers (which I suspect means TUI browsers like Lynx):

For non-Unix platforms, or when a remote browser is available on Unix, the controlling process will not wait for the user to finish with the browser, but allow the remote browser to maintain its own windows on the display. If remote browsers are not available on Unix, the controlling process will launch a new browser and wait.

Do we still need this code, then?

jarun · 2023-12-06T12:54:47Z

Do we still need this code, then?

If it isn't harming anything, let it be.

jarun · 2023-12-08T12:48:07Z

Thank you!

jarun · 2024-01-01T14:34:31Z

@LeXofLeviafan with this change I am hitting the following error on Ubuntu when I start buku:

$ ./buku -h
Traceback (most recent call last):
  File "/home/vaio/github/buku/./buku", line 6058, in <module>
    main()
  File "/home/vaio/github/buku/./buku", line 5284, in main
    colorama.just_fix_windows_console()  # noop on non-Windows systems
AttributeError: module 'colorama' has no attribute 'just_fix_windows_console'

jarun · 2024-01-01T14:58:36Z

Fixed the error at commit 8668ef5. Please review.

LeXofLeviafan · 2024-01-01T15:24:56Z

@jarun the code works on Linux. The issue is caused by the lack of a version requirement (>=0.4.6)

jarun · 2024-01-01T16:00:47Z

Can you please raise a PR?

LeXofLeviafan · 2024-01-01T16:37:51Z

Can you please raise a PR?

#706

[jarun#704] fixed Windows support

d900c98

LeXofLeviafan commented Dec 5, 2023

View reviewed changes

tests/test_bukuDb.py

return bdbs[-1]

yield _bukuDb

rmdb(*bdbs)

Copy link

Collaborator Author

LeXofLeviafan Dec 5, 2023

Choose a reason for hiding this comment

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

This is run after the test is completed

LeXofLeviafan commented Dec 5, 2023

View reviewed changes

jarun merged commit ab03baf into jarun:master Dec 8, 2023
1 check passed

github-actions bot locked and limited conversation to collaborators Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jarun#704] fixed Windows support #705

[jarun#704] fixed Windows support #705

LeXofLeviafan commented Dec 5, 2023 •

edited

Loading

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023 •

edited

Loading

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023

LeXofLeviafan Dec 5, 2023 •

edited

Loading

LeXofLeviafan commented Dec 5, 2023 •

edited

Loading

jarun commented Dec 6, 2023

jarun commented Dec 8, 2023

jarun commented Jan 1, 2024

jarun commented Jan 1, 2024

LeXofLeviafan commented Jan 1, 2024 •

edited

Loading

jarun commented Jan 1, 2024

LeXofLeviafan commented Jan 1, 2024

		except ImportError:
		import pyreadline3 as readline # type: ignore

		@@ -0,0 +1,2 @@
		@if not defined BASEPYTHON set BASEPYTHON=python
		@tox.exe %*

[jarun#704] fixed Windows support #705

[jarun#704] fixed Windows support #705

Conversation

LeXofLeviafan commented Dec 5, 2023 • edited Loading

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

LeXofLeviafan Dec 5, 2023 • edited Loading

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

LeXofLeviafan Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

LeXofLeviafan commented Dec 5, 2023 • edited Loading

jarun commented Dec 6, 2023

jarun commented Dec 8, 2023

jarun commented Jan 1, 2024

jarun commented Jan 1, 2024

LeXofLeviafan commented Jan 1, 2024 • edited Loading

jarun commented Jan 1, 2024

LeXofLeviafan commented Jan 1, 2024

LeXofLeviafan commented Dec 5, 2023 •

edited

Loading

LeXofLeviafan Dec 5, 2023 •

edited

Loading

LeXofLeviafan Dec 5, 2023 •

edited

Loading

LeXofLeviafan commented Dec 5, 2023 •

edited

Loading

LeXofLeviafan commented Jan 1, 2024 •

edited

Loading