ci: bring ci files up to date

.ci/release

@@ -21,15 +21,15 @@ import shutil
 
 is_ci = os.environ.get('CI') is not None
 
-def main():
+def main() -> None:
     import argparse
     p = argparse.ArgumentParser()
     p.add_argument('--test', action='store_true', help='use test pypi')
     args = p.parse_args()
 
     extra = []
     if args.test:
-        extra.extend(['--repository-url', 'https://test.pypi.org/legacy/'])
+        extra.extend(['--repository', 'testpypi'])
 
     root = Path(__file__).absolute().parent.parent
     os.chdir(root) # just in case
@@ -42,7 +42,7 @@ def main():
     if dist.exists():
         shutil.rmtree(dist)
 
-    check_call('python3 setup.py sdist bdist_wheel', shell=True)
+    check_call(['python3', '-m', 'build'])
 
     TP = 'TWINE_PASSWORD'
     password = os.environ.get(TP)

.ci/run

@@ -37,4 +37,4 @@ if ! command -v python3 &> /dev/null; then
 fi
 
 "$PY_BIN" -m pip install --user tox
-"$PY_BIN" -m tox
+"$PY_BIN" -m tox --parallel --parallel-live "$@"

.github/workflows/main.yml

@@ -10,18 +10,22 @@ on:
   pull_request: # needed to trigger on others' PRs
   # Note that people who fork it need to go to "Actions" tab on their fork and click "I understand my workflows, go ahead and enable them".
   workflow_dispatch: # needed to trigger workflows manually
-  # todo cron?
+    # todo cron?
+    inputs:
+      debug_enabled:
+        type: boolean
+        description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
+        required: false
+        default: false
 
 
 jobs:
   build:
     strategy:
+      fail-fast: false
       matrix:
-        platform: [ubuntu-latest, macos-latest] # , windows-latest]
-        python-version: ['3.7', '3.8', '3.9', '3.10', '3.11']
-        # 3.11 on windows has this bug, lxml setup fails
-        #https://bugs.launchpad.net/lxml/+bug/1977998
-        exclude: [{platform: windows-latest, python-version: '3.11'}]
+        platform: [ubuntu-latest, macos-latest]  #, windows-latest]
+        python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
         # vvv just an example of excluding stuff from matrix
         # exclude: [{platform: macos-latest, python-version: '3.6'}]
 
@@ -31,56 +35,61 @@ jobs:
     # ugh https://github.com/actions/toolkit/blob/main/docs/commands.md#path-manipulation
     - run: echo "$HOME/.local/bin" >> $GITHUB_PATH
 
-    - uses: actions/setup-python@v4
+    - uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python-version }}
 
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
       with:
         submodules: recursive
+        fetch-depth: 0  # nicer to have all git history when debugging/for tests
 
-    # uncomment for SSH debugging
-    # - uses: mxschmitt/action-tmate@v3
+    - uses: mxschmitt/action-tmate@v3
+      if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled }}
 
     # explicit bash command is necessary for Windows CI runner, otherwise it thinks it's cmd...
     - run: bash .ci/run
 
     - if: matrix.platform == 'ubuntu-latest'  # no need to compute coverage for other platforms
-      uses: actions/upload-artifact@v3
+      uses: actions/upload-artifact@v4
       with:
         name: .coverage.mypy_${{ matrix.platform }}_${{ matrix.python-version }}
         path: .coverage.mypy/
 
+
   ###
   build_extension:
     env:
       name: 'grasp'
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
       with:
         submodules: recursive
-    - uses: actions/setup-node@v3
+        fetch-depth: 0  # nicer to have all git history when debugging/for tests
+
+    - uses: actions/setup-node@v4
       with:
-        node-version: '18'
+        node-version: '20'
+
     - run: extension/.ci/build --lint # debug version
     - run: extension/.ci/build --lint --release
 
     # TODO ugh. can't share github actions artifacts publicly...
     # TODO for fuck's sake... why does it end up named as .zip.zip ????
-    - uses: actions/upload-artifact@v2
+    - uses: actions/upload-artifact@v4
       with:
         name: '${{ env.name }}-chrome-debug-latest.zip'
         path: 'extension/dist/artifacts/chrome/${{ env.name }}_dev_-*.zip'
-    - uses: actions/upload-artifact@v2
+    - uses: actions/upload-artifact@v4
       with:
-        .name : '${{ env.name }}-chrome-release-latest.zip'
+        name : '${{ env.name }}-chrome-release-latest.zip'
         path: 'extension/dist/artifacts/chrome/${{ env.name }}-*.zip'
-    - uses: actions/upload-artifact@v2
+    - uses: actions/upload-artifact@v4
       with:
         name : '${{ env.name }}-firefox-debug-latest.zip'
         path: 'extension/dist/artifacts/firefox/${{ env.name }}_dev_-*.zip'
-    - uses: actions/upload-artifact@v2
+    - uses: actions/upload-artifact@v4
       with:
         name : '${{ env.name }}-firefox-release-latest.zip'
         path: 'extension/dist/artifacts/firefox/${{ env.name }}-*.zip'

extension/build

@@ -35,6 +35,8 @@ def main():
 
     assert target is not None
 
+    manifest = args.manifest or '3'
+
     base_ext_dir = Path(__file__).absolute().parent / 'dist'
     ext_dir = (base_ext_dir / target).resolve() # webext can't into symlinks
     # sadly no way to specify zip name in the regex..
@@ -52,7 +54,7 @@ def main():
         'TARGET' : target,
         'RELEASE': 'YES' if args.release else 'NO',
         'PUBLISH': 'YES' if args.publish else 'NO',
-        'MANIFEST': args.manifest or '2',  # TODO 2 is default for now, but make required later?
+        'MANIFEST': manifest,
         'EXT_ID' : IDS[target],
         **os.environ,
     }

extension/webpack.config.js

@@ -112,18 +112,19 @@ const background = {}
 if (v3) {
   if (target === T.CHROME) {
     background['service_worker'] = 'background.js'
+    background['scripts'] = [
+      'background.js',
+    ]
     // see header of background.js, this was for some experiments
     // NOTE: not working in firefox? just fails to load the manifest
     // background['type'] = 'module'
   } else {
     background['scripts'] = [
-      'webextension-polyfill.js',
       'background.js',
     ]
   }
 } else {
   background['scripts'] = [
-    'webextension-polyfill.js',
     'background.js',
   ]
   background['persistent'] = false
@@ -151,12 +152,11 @@ if (v3) {
   // FIXME not sure if firefox supports this??
   // https://bugzilla.mozilla.org/show_bug.cgi?id=1766026
   manifestExtra['optional_host_permissions'] = optional_host_permissions
-  if (target === T.FIREFOX) {
-    manifestExtra['browser_specific_settings'] = {
-      "gecko": {
-        "id": ext_id,
-      },
-    }
+  const gecko_id = target === T.FIREFOX ? ext_id : "{00000000-0000-0000-0000-000000000000}"
+  manifestExtra['browser_specific_settings'] = {
+    "gecko": {
+      "id": gecko_id,
+    },
   }
 } else {
   manifestExtra.permissions.push(...host_permissions)

mypy.ini

@@ -1,9 +1,14 @@
 [mypy]
+namespace_packages = True
 pretty = True
 show_error_context = True
 show_error_codes = True
+show_column_numbers = True
+show_error_end = True
+warn_unused_ignores = True
 check_untyped_defs = True
-namespace_packages = True
+enable_error_code = possibly-undefined
+strict_equality = True
 
 # an example of suppressing
 # [mypy-my.config.repos.pdfannots.pdfannots]

ruff.toml

@@ -0,0 +1,25 @@
+lint.ignore = [
+### too opinionated style checks
+    "E501",  # too long lines
+    "E702",  # Multiple statements on one line (semicolon)
+    "E731",  # assigning lambda instead of using def
+    "E741",  # Ambiguous variable name: `l`
+    "E742",  # Ambiguous class name: `O
+    "E401",  # Multiple imports on one line
+    "F403",  # import *` used; unable to detect undefined names
+###
+
+###
+    "E722",  # Do not use bare `except` ## Sometimes it's useful for defensive imports and that sort of thing..
+    "F811",  # Redefinition of unused  # this gets in the way of pytest fixtures (e.g. in cachew)
+
+## might be nice .. but later and I don't wanna make it strict
+    "E402",  # Module level import not at top of file
+
+### maybe consider these soon
+# sometimes it's useful to give a variable a name even if we don't use it as a documentation
+# on the other hand, often is a sign of error
+    "F841",  # Local variable `count` is assigned to but never used
+    "F401",  # imported but unused
+###
+]

server/grasp_server.py

@@ -155,7 +155,7 @@ def setup_parser(p):
 def main():
     logging.basicConfig(level=logging.DEBUG, format='%(asctime)s %(name)-12s %(levelname)-8s %(message)s')
 
-    p = argparse.ArgumentParser('grasp server', formatter_class=lambda prog: argparse.ArgumentDefaultsHelpFormatter(prog, width=100)) # type: ignore
+    p = argparse.ArgumentParser('grasp server', formatter_class=lambda prog: argparse.ArgumentDefaultsHelpFormatter(prog, width=100))
     setup_parser(p)
     args = p.parse_args()
     run(args.port, args.path, args.template, args.config)

server/test_with_browser.py

@@ -7,7 +7,7 @@
 import json
 import os
 
-import pytest # type: ignore
+import pytest
 from selenium import webdriver # type: ignore
 
 
@@ -85,7 +85,7 @@ def trigger_grasp():
     # pyautogui.locateOnScreen('/L/soft/browser-extensions/grasp/unicorn.png')
 
     print("sending hotkey!")
-    import pyautogui # type: ignore
+    import pyautogui
     pyautogui.hotkey('ctrl', 'alt', 'c')
 
 

setup.py

@@ -37,7 +37,7 @@ def main() -> None:
         ],
         extras_require={
             'testing': ['pytest', 'requests'],
-            'linting': ['pytest', 'mypy', 'lxml'], # lxml for mypy coverage report
+            'linting': ['pytest', 'mypy', 'lxml', 'ruff'], # lxml for mypy coverage report
         },
 
 

tox.ini

@@ -1,32 +1,49 @@
 [tox]
-minversion = 3.5
+minversion = 3.21
 # relies on the correct version of Python installed
-envlist = tests,mypy
+envlist = ruff,tests,mypy
 # https://github.com/tox-dev/tox/issues/20#issuecomment-247788333
 # hack to prevent .tox from crapping to the project directory
-toxworkdir={env:TOXWORKDIR_BASE:}{toxinidir}/.tox
+toxworkdir = {env:TOXWORKDIR_BASE:}{toxinidir}/.tox
 
 [testenv]
-passenv = 
+# TODO how to get package name from setuptools?
+package_name = "server"
+passenv =
 # useful for tests to know they are running under ci
-  CI
-  CI_*
+    CI
+    CI_*
 # respect user's cache dirs to prevent tox from crapping into project dir
-  MYPY_CACHE_DIR
-  PYTHONPYCACHEPREFIX
+    PYTHONPYCACHEPREFIX
+    MYPY_CACHE_DIR
+    RUFF_CACHE_DIR
+
+
+# note: --use-pep517 below is necessary for tox --parallel flag to work properly
+# otherwise it seems that it tries to modify .eggs dir in parallel and it fails
+
+
+[testenv:ruff]
+commands =
+    {envpython} -m pip install --use-pep517 -e .[linting]
+    {envpython} -m ruff check server/
+
 
 [testenv:tests]
 commands =
-    pip install -e .[testing]
+    {envpython} -m pip install --use-pep517 -e .[testing]
     # posargs allow test filtering, e.g. tox ... -- -k test_name
-    python -m pytest server --ignore server/test_with_browser.py {posargs}
+    {envpython} -m pytest \
+        'server' --ignore 'server/test_with_browser.py' \
+        {posargs}
+
 
 [testenv:mypy]
 commands =
-    pip install -e .[linting]
-    python -m mypy --install-types --non-interactive \
-                   server                            \
-                   # txt report is a bit more convenient to view on CI
-                   --txt-report  .coverage.mypy      \
-                   --html-report .coverage.mypy      \
-                   {posargs}
+    {envpython} -m pip install --use-pep517 -e .[linting]
+    {envpython} -m mypy --install-types --non-interactive \
+        'server'       \
+        # txt report is a bit more convenient to view on CI
+        --txt-report  .coverage.mypy     \
+        --html-report .coverage.mypy     \
+        {posargs}