| Age | Commit message (Collapse) | Author |
|---|
|
The type hint one could probably resolved with annotations from the
future. The inspector/miscwidgets one will probably need refactoring.
|
|
these will probably conflict with the recent PyQt stubs changes
|
|
|
|
pylint told me to fix them.
Presumably the utils.py one confused isort with the try/catch above it
|
|
Unfortunately isort seems to run on all imports in modified files, instead of
just modified ones. Possibly there is some way to tell it to only touch
`qutebrowser.qt` imports but I couldn't find it (or to filter the diff
output). So there is a bit more noise in this one than the black one.
It might be helpful to try to exclude it from running on some files (like
`qutebrowser/qt.py`) using noqa directives or something.
I had to tell it our source directories explicitly else it moved `from
helpers` up in the test files.
It did touch imports in `if TYPE_CHECKING:` blocks, it didn't touch stuff in
`if PyQt6:` blocks. I guess I had mypy installed, and not PyQt6.
There are a couple of options that can reduce the diff. They cause
pylint to be unhappy though:
length_sort_straight = true
Causes string imports to be sorted by length (eg os, sys, importlib)
instead of alphabetically. This fits with the existing style better but
it causes, for example, `os` and `os.path` to be sorted differently
which makes pylint complain.
length_sort = true
Similar to the above option but for all imports and import aliases.
Import aliases in particular are not currently sorted so it doesn't end
up reducing the diff substantially.
only_sections = true
Causes isort to not re-order imports, only move them between sections.
Which is great for reducing the diff but it doesn't put the moved
imports into a well sorted place, it just puts them at the top/bottom of
the section they are moved to. So, for the tests in particular, it was
ending up with `qutebrowser.qt`, `helpers`, `qutebrowser.browser...`
which also made pylint complain about ungrouped imports.
It would be great if I could do `only_sections` and then tell it to only
sort the first party section. But sorting only specific sections doesn't
seem to be an option!
ran like:
$ cat pyproject.toml
[tool.black]
skip-string-normalization = true
target-version = 'py37'
[tool.isort]
#length_sort_straight = true
group_by_package = true
#only_sections = true
profile = 'black'
honor_noqa = true
src_paths = ["qutebrowser", "tests"]
$ darker -i -W 8 -r 488dc175e069 qutebrowser tests/
|
|
Running darker over the pyqt reference renames (see previous commit)
worked pretty well. Unfortnately we ran into
https://github.com/psf/black/issues/620 a lot with dicts with long
attributes. Things that used to be manually wrapped like:
some_map = {
quote_long_attribute_name:
also_long_value,
}
get turned into:
some_map = {
quote_long_attribute_name: also_long_value,
}
which is quite often over 88 chars with Qt enum values in the map.
I've manually added aliases and such for the Qt enums and classes to
shorted the lines.
This is likely to conflict entirely with the enum scoped changes on the
qt6 branches.
|
|
Looks like there is still a few places like:
thing = {
really.hecking.long.name = foo,
}
where black is unable to reformat them to be under 88 chars. Possibly
adding backslashes would help, I might manually edit them to have part
of the right hand side referred to via a shorter local variable.
ran like:
$ cat pyproject.toml
[tool.black]
skip-string-normalization = true
target-version = 'py37'
darker diff -W 8 -r 488dc175e069 qutebrowser tests/
|
|
"I open" in the BDD tests can match against a message like this:
01:24:53.227 DEBUG downloads qtnetworkdownloads:fetch:563 fetch: PyQt5.QtCore.QUrl('http://localhost:58917/500-inline') -> attachment.jpg
The QUrl class name there still references the specific PyQt module, so
make the match expression less specific.
Similar to 36763864 for the scroll position e2e tests.
Caused by the regex rename in 80033f466f96a
|
|
|
|
adjust indent of trailing lines because the leading ones got longer when
re-writing imports in 46d426a6519e1
|
|
a few directives that got lost when adapting stuff for a pyqt wrapper
|
|
The LibCST rename tool got a bit confused by imports done in `if
TYPE_CHECKING:` blocks. For ones that were used it moved them out of
those blocks. For ones that were only used in string type hints it half
changed the imports and didn't updated the strings.
A couple of instances were fixed in previous commits. These are the last
few that mypy could see.
Addressed manually. These fixers based on LibCST
https://github.com/python-qt-tools/PyQt6-stubs/tree/main/fixes look like
they could be useful. But they don't have any documentations of tests or
anything. So it's much faster to fix these manually then to try to
understand what all that is doing, if any of it is useful for us,
extract it, test it etc.
|
|
I did a mass rename of PyQt5 -> qutebrowser.qt in 80033f466f96a but in
this instance the underlying canonical QPoint name is logged. So change
these back but wildcard the module name so hopefully it'll work with
PyQt5 and 6.
|
|
I had this in earlier but then didn't seem to need it. Now CI is
failing.
I though I had it with a WebKit install that had PyQtWebEngine installed
as well (but not the C++ QtWebEngine). It could install PyQtWebEngine
but didn't have anything in it.
So if this works it might be a sign that more stuff will trip over it to
come. For example `from PyQt5 import QtWebEngine` might work but `from
PyQt5.QtWebEngine import PYQT_WEBENGINE_VERSION_STR` wouldn't. So we
might be saddling ourselves with a false sense of security just
switching on the top level stuff in the PyQt wrapper class.
|
|
Partially corrects "Manual fixups for ImportErrors lost in rewrite"
Instead of looking for import errors looks for attributes being None.
have conditional defaults.
In qlocalsocket_mock I had to move the original attribute gathering to even
earlier otherwise we were just picking up mocks.
|
|
Now that we are importing the PyQt modules and then accessing stuff as
attributes on them the mocks in the tests should be done the same way.
This was done manually, I didn't even try to automate it.
|
|
Mostly replaces "68007002fc64 Add pytest.importorskip wrapper", but
after import strings got renamed in "1faab7e5609e Update string PyQt5
references in tests".
pytest.importskip uses `__import__('qutebrowser.qt.QtCore')` etc. Which
doesn't work without QtCore being it's own file in the qutebrowser.qt
package. That's not what we are going for here (if it was we wouldn't
have to re-write all the references!).
So I've replaced the previous attempt with something that'll just look
for attributes on the qutebrowser.qt module.
Additionally there were some imports that were just done to get the
module inside a function, and guard. I've changed them to two lines, our
custom one with a relevent Qt module as a guard and then
pytest.importorskip to get the actual module they want.
|
|
git grep -l "['\"]PyQt5" tests/ | xargs sed -i -e 's/"PyQt5/"qutebrowser.qt/' -e "s/'PyQt5/'qutebrowser.qt/"
|
|
These require a bit of special casing due to how they handle imports.
|
|
I forgot to include QtTest in the renamer it seems.
Import in TYPE_CHECKING conditions are not consistently handled, possibly when
they are only used in string types.
Not sure what happened with the duplicates in the logging modules.
Found by grepping PyQt5
|
|
Now that we are importing all of the PyQt modules from the same places
raising an ImportError when an import fails would be inconvenient for
people trying to run the browser with only one backend available.
Instead they are set to `None`. This adjusts the PyQt related
importorskip directives to use our wrapper that'll also skip if the
target is `None`.
I don't expect pylint to be happy about the import orders.
I haven't tested without webengine available (and with webkit).
|
|
I super don't like the extra duplication but we are all servants of our
linters.
Making mypy happy is the main reason I'm going through this renaming
stuff effort so here it is.
Remaining failures are because of string types that didn't get renamed
and imports in TYPE_CHECKING conditionals.
Options for for fixing those are:
* running the rename tool again with TYPE_CHECKING=True everywhere
* scraping mypys errors for things to update and;
* regular expressions
|
|
Some test imports that didn't have the things there were importing used
got dropped by the rename tool. This restores them.
Found by searching in the diff for eg `import qutebrowser.qt$` and
`import qutebrowser.qt `
Relates to #995
|
|
Fixups for commit PyQt import rewrite commit "Re-write PyQt5 imports to be via our wrapper."
Import not at the top of the file where mostly moved up there. Where
they were inside a try/except block they were replaced with `pass`
(why!). With the current setup of having everything imported into qt.py
we don't get import errors anymore anyway. So affected places have been
changed to check for None.
This has not currently been well tested. We might have to move some of
the conditional checks into qt.py.
I identified these pieces of code by looking for `^+ * pass$` in the
diff.
|
|
Import all the PyQt modules that we use into qt.py so that we only have to
have a PyQt5/6 conditional in one place.
I'm using importlib instead of try/excepts to reduce the amount of duplication
and because you can't do `from pyqt import QtCore` when pyqt is a local
variable.
TODO:
* maybe some more conditional imports (for earlyinit stuff like no QSsl
support)
* adapt places looking for import errors to just importing None now
* decide whether being in qt.py is fine or we should move to `qt/__init__.py`
|
|
Still some manual fixes to make, see #995 and linked PRs.
This was done like:
date ; for mod in QtCore QtDBus QtGui QtNetwork QtPrintSupport QtQml QtSql QtWebEngine QtWebEngineCore QtWebEngineWidgets QtWebKit QtWebKitWidgets QtWidgets; do
echo "renaming $mod"
python3 -m libcst.tool codemod rename_pyqt.RenameCommand qutebrowser/ tests --old_name=PyQt5.$mod.* --new_name=qutebrowser.qt:$mod.* --no-format
done ; dat
Where the list of modules was obtained via:
semgrep --lang=py -e 'from PyQt5.$SUBMODULE import ($IMPORTEES)' -o findings.json --json qutebrowser scripts/ tests/
and then
with open("findings.json") as f: data=json.load(f)
results = data['results']
submodules = sorted(set(r['extra']['metavars']['$SUBMODULE']['abstract_content'] for r in results))
for m in submodules: print(m)
|
|
(From my LibCST fork e6b990836204b)
The rename codemod currently has a few variables cached on `self` which
are used to transfer information between different visitor methods.
For example one of the nodes being visited inside an import from
statement might cause the import to be skipped in `leave_Import` using
`self.bypass_import`.
Or in the test case below relevant seen imports are cached in
`self.scheduled_removals` so they can be given special attention in
`leave_Module`.
The lifecycles don't work out though as the transforms that codemods run
ass are only initialised once for a whole run. So stuff can leak between
modules. There is probably a better place for them, but this works for
now. I'm not sure if this is reproducable outside of this wildcard
rename support.
Here is an example:
1. Have a folder with two test files
==> context_test/1.py <==
from a.b import c
c.x.foo
==> context_test/2.py <==
from a.b.g import d
d.bar
2. Rename something from the first file
python3 -m libcst.tool codemod rename.RenameCommand context_test/ -j 1 --old_name=a.b.c.* --new_name=a.b:c.* --no-format ; head -n-0 context_test/*
3. Observe a removed import from the first file leaks into the second
==> context_test/1.py <==
from a.b import c
c.x.foo
==> context_test/2.py <==
from a.b.g import d
from a.b import c
d.bar
|
|
When the target rename was something like "a.b.c" and the file happened
to do a simple "import a" it would fail with `CSTValidationError:
Cannot have empty name identifier.` when trying to construct a new
import alias. Because the result of gen_replacement_module() in this
case was an empty string. Which is apparently used for "no replacement
required", which is accurate in this case.
This is a bandaid, the problem is more likely to be that the
`old_name.startswith(import_alias_full_name+'.')` conditional is too
simplistic.
|
|
Fix this https://github.com/Instagram/LibCST/pull/675#issue-1198913564
And supports wildcard renaming. So that we don't have to run it for
every attribute of every PyQt module we import. Just for every PyQt
module we import. (We can change it to hardcode them since this copy is
just in this repo!)
|
|
It works! With lots of printf debugging!
Now that I've gone through and made it work I need to go back and try to
understand it and think about all the cases I haven't tested and might
have broken.
This is my minimal example:
before:
from qutebrowser.qt.QtCore import (QUrl,
QPoint)
from qutebrowser.qt.QtGui import QIcon
foo = QUrl("about:blank")
bar = QPoint()
qux = QIcon()
after:
from qutebrowser.qt import QtCore, QtGui
foo = QtCore.QUrl("about:blank")
bar = QtCore.QPoint()
qux = QtGui.QIcon()
I've already ran into one case that I didn't cover with that:
from qutebrowser.qt.QtCore import Qt
foo = Qt.Key_Enter
|
|
Copies the upstream rename codemod because it has shown pretty good
results but I want to change it to support wildcards. For example:
python -m libcst.tool codemod rename.RenameCommand qutebrowser/browser/browsertab.py --old_name=qutebrowser.qt.QtCore.pyqtSignal --new_name=qutebrowser.qt:QtCore.pyqtSignal --no-format
With the vanilla codemod correctly handles the pyqtSignal import and all
the references but I don't want to have to run it for every Qt type. I
would like to be able to specify names like:
--old_name=qutebrowser.qt.QtCore.* --new_name=qutebrowser.qt:QtCore.*
|
|
|
|
|
|
.width() was deprecated in Qt 5.11 because what it actually does is better
described as .horizontalAdvance():
https://codereview.qt-project.org/c/qt/qtbase/+/201397
(ee2ad9df701b27790e2ab72e99111d255fde42ed in qtbase)
Docs:
https://doc.qt.io/qt-6/qfontmetrics.html#horizontalAdvance-2
What we actually want for the size hint is the bounding box.
(originally cherry picked from commit 78838338c331d74931da31acd2306550721ef121)
|
|
This got moved to QSslConfiguration in Qt 5.5:
https://codereview.qt-project.org/c/qt/qtbase/+/113886
(92cda9474245c79b635c21cd140c5d0a3a6d2e5b in qtbase)
(cherry picked from commit 317da1e3cf23bf40d24d186cd6d06b6bc9a09958)
|
|
(cherry picked from commit 18d7e400cd7f61647953f1f1b5e74006246d4276)
|
|
Update dependencies
|
|
|
|
|
|
|
|
|
|
|
|
Fixes #7108
|
|
For Makefile installs (broke while copying stuff over) and pyinstaller
installs (broke on launch).
|
|
|
|
Follow-up for #6905
|
|
|
|
This reverts commit 2b76b6164093754482d848e7487356215556d0d0.
|
|
|
|
|
