The lint ones are:
linters (pylint): qutebrowser/completion/completionwidget.py#L440
Consider using 'height = min(height, contents_height)' instead of unnecessary if block
linters (pylint): qutebrowser/browser/webengine/webview.py#L241
Useless suppression of 'no-member'
The no-member one might be due to this change: https://github.com/pylint-dev/pylint/issues/9246
For the importlib-resources one, I'm not sure why it's changed to be
underscore instead of a dash now. But at least it's consistent across
all the requirements files now!
I feel like I've seem this in a previous dependancy update too (for a
different package?) but I can't find that now.
In `pylint_checkers/setup.py` we define the package name as `qute_pylint`. When
setuptools was creating the egg (or wheel?) it was sanitizing the package name
replacing all problematic chars, including underscore, with a hyphen (-). So
the output of `pip freeze`, among other things, have the name with a hyphen:
`qute-pylint`. This was fixed in setuptools so underscores are no longer
sanitized: https://github.com/pypa/setuptools/issues/2522
Change the regex to include both just in case someone is running it with the
old setuptools or something. Also because I'm not able to reproduce the
hyphen version of the name locally, not sure why. Maybe it depends on your
python or importlib version too?
I added isort!=5.13.0 in misc/requirements/requirements-pylint.txt and
ran `python3 scripts/dev/recompile_requirements.py pylint` in `docker
run -v `pwd`:/work -w /work -it python:3.8-bookworm bash` based off of
main.
Then reverted the relevant part of the update dependencies commit with
`git show HEAD misc/requirements/requirements-pylint.txt | patch -R -p
1` and manually updated the two deps from the local update run :)
diff --git i/misc/requirements/requirements-pylint.txt w/misc/requirements/requirements-pylint.txt
index 80319adc03a7..1c8fa33e623a 100644
--- i/misc/requirements/requirements-pylint.txt
+++ w/misc/requirements/requirements-pylint.txt
@@ -11,7 +11,7 @@ idna==3.6
isort==5.12.0
mccabe==0.7.0
pefile==2023.2.7
-platformdirs==4.0.0
+platformdirs==4.1.0
pycparser==2.21
PyJWT==2.8.0
pylint==3.0.2
@@ -21,6 +21,6 @@ requests==2.31.0
six==1.16.0
tomli==2.0.1
tomlkit==0.12.3
-typing_extensions==4.8.0
+typing_extensions==4.9.0
uritemplate==4.1.1
# urllib3==2.1.0
ref: https://github.com/qutebrowser/qutebrowser/pull/8028#issuecomment-1849363267
I believe we are being afflicted by this issue: https://github.com/python/mypy/issues/16451
Although I'm not 100% sure because there is a lot going on in this
function and I haven't managed to grok it.
The mypy 1.7 release [notes][1.7] say you can disable the new type
inference by running `tox -e mypy-pyqt6 -- --old-type-inference` and
indeed mypy passes with that. So either our type hints are incorrect or
we are hitting a bug. Considering the inferred type hint has a `Never`
in it I'm leading toward it being a bug. So I'll bump the mypy version
down and hopefully next week the issue will be resolved.
The mypy output before this commit was:
mypy-pyqt6: commands[0]> .tox/mypy-pyqt6/bin/python -m mypy --always-true=USE_PYQT6 --always-false=USE_PYQT5 --always-false=USE_PYSIDE6 --always-false=IS_QT5 --always-true=IS_QT6 --always-true=IS_PYQT --always-false=IS_PYSIDE qutebrowser
qutebrowser/utils/qtutils.py:239: error: Argument 1 to "contextmanager" has incompatible type
"Callable[[str, bool, str], Iterator[IO[AnyStr]]]"; expected "Callable[[str, bool, str], Iterator[IO[Never]]]" [arg-type]
@contextlib.contextmanager
^
qutebrowser/misc/lineparser.py: note: In member "save" of class "LineParser":
qutebrowser/misc/lineparser.py:168: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._configfile, self._binary) as f:
^
qutebrowser/misc/lineparser.py: note: In member "save" of class "LimitLineParser":
qutebrowser/misc/lineparser.py:226: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._configfile, self._binary) as f:
^
qutebrowser/config/configfiles.py: note: In member "_save" of class "YamlConfig":
qutebrowser/config/configfiles.py:292: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._filename) as f:
^
qutebrowser/misc/sessions.py: note: In member "save" of class "SessionManager":
qutebrowser/misc/sessions.py:343: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(path) as f:
[1.7]: https://mypy-lang.blogspot.com/2023/11/mypy-17-released.html
1. run jseval in main world: the script adds an element that calls the
`restore_video` function. This was failing with a "not found" message
on webengine, presumably because the dom click handler runs in the
main world and the function was over in the jseval world. The the
script predates webengine which is the backend that implements the
worlds.
2. remove a console log message, seems to be just noise and easy enough
to add back later
3. remove href attribute of the restore video link: this seemed to be
causing the `restore_video` method to be called twice. The second
time with `this` as the global Window object, which was causing an
error because that has a null `parentNode` attribute.
4. added the `cursor: pointer` style that was needed since the element
didn't have an href anymore
5. change the mpv flags `--terminal` -> `--quiet`. This means we get
error messages (eg from yt-dlp) in error logs and in the `:process`
page now. It can get a bit spammy though if you are running from a
terminal. I'm getting a log of keepalive warning and error logs from
ffmpeg. On the other hand it's really annoying to see a "process
failed, see :process for details" and having no error messages in
there.
Fixes: #7838
The CheckPlatform macro will prompt the user user to use the 32bit installer
if they are on a 32bit system. But we don't provide a 32bit installer anymore.
This commit changes the OS version check for Qt5 builds to be based on checking
version numbers ourselves too, so that we can have our own error message.
Also moves the Qt5 conditionals to be compile time ones.
We dropped 32bit support in #7804 and as a result removed the arch
suffix from the binary that pyinstaller produces. This commit removes it
form the lookup path in the installer too.
Note that we are leaving the arch string in the installer itself for
now. Mostly because it'll be removed as part of a later change when the
installer itself is refreshed. But it might also be useful to clarify in
the installer names what the arch is? Maybe, that reasoning might not
fit with the previous change to remove the arch strings.
The Qt docs for 6.5 say that the minimum supported version is Windows 10
1809.
Experimentally it seems qutebrowser and it's dependencies work fine on a
version as early 1607.
There should be no change in OS version requirements for the Qt5 build,
although we've dropped 32 bit support already and in a future version of
the installer we may bring the minimum OS version support in line with
the Qt6 requirements for simplicity too.
Added a new QT5 version into the NSIS scripts so we can do the different
version check per installer build. It just uses the python bool
serialization format so should always be "True" or "False", but I've
added a fallback anyway for consistency.
I don't want to deal with having to review development changes of
pyinstaller every week. So pin to one commit for now that we can
actually test. I'm subscribed to release notifications on github so I'll
manually change this back to point to the pyinstaller pypi package ones
it does.
Not that we are looking up resources via importlib_resources for
pyinstaller builds too we need to change where the data files are
installed to to match what importlib_resources is expecting.
There was a comment in the previous resource lookup special case
complaining about the data files being at the top level so it seems this
is a change for the better anyhow.
Observed paths:
requested file: qutebrowser.app/Contents/Frameworks/qutebrowser/config/configdata.yml
actual file : qutebrowser.app/Contents/Frameworks/config/configdata.yml
PyInstaller has [recently][symlinks] landed a change that should restore
the webengine sandbox on macOS builds. With a bit more testing we are
hoping that we can go ahead releasing builds based on that
in-development PyInstaller codebase even before they've made a release.
This commit pins our pyinstaller dependency to be the latest commit on
their develop branch.
[symlinks]: https://github.com/pyinstaller/pyinstaller/pull/7619
reuse annotate --license="GPL-3.0-or-later" --style python \
misc/nsis/uninstall_pages.nsh \
misc/nsis/install.nsh \
misc/nsis/uninstall.nsh
And fixing qutebrowser.nsi manually as that uses iso-8859-1 and the reuse tool
doesn't like that apparently.
These tests were failing on webkit due to cookies not getting set (they
made a change recently to fix a security issue around cookies and must
have messed something up)
tests/end2end/test_invocations.py::test_cookies_store
tests/end2end/features/test_private_bdd.py::test_make_sure_private_data_is_not_cleared_when_closing_a_private_window_but_another_remains
Pin Flask and Werkzeug for py3.7
They've removed compatibility upstream.
Added example to readme file because I had to go dig for one in the
commit history.
Add blinker to changelog URLs as it's a required dependency of Flask
now.
The previous way of obtaining the Bang's URL was using the
response's body, using the url atrribute of a meta tag. The way
this was handled was a bit problematic: some URLs were of the format
/l/?uddg=<URL>&<some-other-query-arguments>, while others (such as google)
is simply <URL>. This could have been fixed by using an if-else, but I
preferred another approach. Lite DuckDuckGo sends a 303 response when
using Bangs, instead of a 200 response which makes the redirect. The
location header of the 303 response is the search engine's URL, so it's
easy to implement this function, working for both afformentioned URL
templates.
Looks like the upstream bug is fixed now.
I didn't run recompile-requirements because it requires py3.7 for the
pylint file and I don't have that. The robot will run overnight anyway
(and pick up a new pylint release)
ref: 085a7956eb
The changes in requirements-mypy.txt would get overwritten on the next
dependency update. Also, it looks like we don't actually need PyQt6 (or
the PyQt6 stubs) available for checking PyQt 5 code if all Qt 6 imports
are appropriately gated by conditionals mypy knows about.
Follow-up to c1738ca550.
Would be nice to have a bare `mypy` env which ran both the more specific ones
in sequence but I don't know how to do that.
Not sure if there is a way to pull the CONSTANTS_ARGS stuff out to a non-env
parameter and pass it into commands but I couldn't figure out a way. So via
the environment it is.
TODO: compare PyQt6 as-is with the WIP PyQt6-Stub