Sync addons-source@maintenance/gramps60 with upstream (2026-07-25) - #62
Sync addons-source@maintenance/gramps60 with upstream (2026-07-25)#62eduralph wants to merge 14 commits into
Conversation
Seventeen manual pages for addon authors - overview and getting started, tutorials per addon kind, the addon-kinds catalogue, registration fundamentals, data access, API reference, testing, debugging, troubleshooting, code analysis, internationalization, packaging, post-merge community steps, compatibility, per-release changes, normative guidelines, and roadmap - plus the diagrams they embed and a folder index. The new docs/ tree is invisible to make.py and CI: every enumeration keys on *.gpr.py or *.py globs that match nothing under docs/, verified with manifest-check and a no-op 'build docs' run.
README: the develop-your-own-addon pointer now leads to the in-repo docs/addon-development manual first, with CONTRIBUTING.md for the contributor workflow and the wiki page as an alternative rendering. The dead Travis badge is dropped. CONTRIBUTING: the deep technical sections that the manual now covers - addon kinds, registration and GENERAL plugins, prerequisites, addon configuration, localization, distribution contents, report categories, and the wiki listing/documentation templates - are reduced to a short retained summary plus a link into the manual, each under its original heading so existing deep links keep resolving. The contributor-workflow content that is unique to this document - repository and fork setup, development branches, the addon checklist, the pull-request walkthrough, and the maintenance guidance - is kept in place unchanged. Also repairs pre-existing broken links: seven table-of-contents and overview anchors that never matched their headings, two (#https://...) hrefs, the garbled Addon-list-legend link, and the Localization snippet that had lost its underscore binding. Depends on the PR that adds docs/addon-development.
The addons-source CI (PR 820) adds a lint step that fails on any tracked Python file carrying trailing whitespace. Three pre-existing files trip it (27 lines total): ArchiveAssist/ArchiveAssist.py (22), and two FilterRules modules (5). Strip the trailing whitespace so the gate can pass; whitespace only, no behavioural change (git diff -w is empty).
_build_with_progress used `for _ in range(batch_size)`, which makes _
a local variable throughout the whole method (Python binds it at compile
time). That shadows the module-level `_ = _trans.gettext`, so:
- the except handler at the top of the method, LOG.error(_("Error
initializing data: %s") % str(e)), runs before the loop assigns _
and raises UnboundLocalError; and
- the in-loop handler LOG.warning(_("Error processing person %s: %s"
% ...)) calls _ after the loop bound it to an int, raising TypeError.
Both fire only on the error paths, so they slipped through. The loop
counter is unused; rename it to _batch_step so _ resolves to the module
gettext everywhere. ruff (E9,F63,F7,F82) is clean on the module.
This also clears the F82x lint error PR 820's ruff gate reports on the
current tree.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f1011fa25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| register( | ||
| QUICKVIEW, | ||
| id="Siblings", |
There was a problem hiding this comment.
On Gramps 6.0, the injected registration constant is QUICKREPORT, not QUICKVIEW; every working Quick View in this branch, such as AllNamesQuickview/AllNames.gpr.py, uses QUICKREPORT. Following this tutorial therefore raises NameError while loading the .gpr.py, so the example never appears in the UI.
Useful? React with 👍 / 👎.
|
|
||
| 2. **`gramps_target_version` mismatch.** A `6.0` addon won't load in 6.1, and vice versa. Plugin discovery silently skips the registration entry. See [14-compatibility → `gramps_target_version` semantics](14-compatibility.md#gramps_target_version-semantics). | ||
|
|
||
| 3. **`id` doesn't match the folder name.** The addon's folder name and the `id` argument to `register(...)` must be identical. Gramps does not match by content — it matches by folder name and verifies against `id`. A mismatch silently drops the entry. |
There was a problem hiding this comment.
Plugin IDs need to be unique but do not have to equal the containing folder name, as this manual correctly states elsewhere and existing addons demonstrate (for example, folder AllNamesQuickview registers id="allnames"). Treating a mismatch as a loading failure sends users toward an unnecessary rename while overlooking the real registration error.
Useful? React with 👍 / 👎.
|
|
||
| 3. **`id` doesn't match the folder name.** The addon's folder name and the `id` argument to `register(...)` must be identical. Gramps does not match by content — it matches by folder name and verifies against `id`. A mismatch silently drops the entry. | ||
|
|
||
| The fastest check: in a Python REPL with `gramps` on `sys.path`, `exec(open("MyAddon/MyAddon.gpr.py").read())`. If it raises, you have your cause; if it returns silently and there's no entry, your `register()` call is being filtered out. |
There was a problem hiding this comment.
A plain exec in a REPL does not provide the names that Gramps injects into .gpr.py, including register, _, GRAMPLET, and the status constants. Consequently this suggested check raises NameError for every valid registration file and falsely reports a registration defect; the diagnostic must supply make_environment/a stub register, or use the existing registration checker.
Useful? React with 👍 / 👎.
|
|
||
| # 1. Iterate: edit, restart Gramps, verify behaviour. | ||
| # 2. Refresh translations if user-visible strings changed. | ||
| make.py gramps60 update <Addon> all |
There was a problem hiding this comment.
make.py does not support all for the update command: it assigns the fifth argument directly to locale and requires <Addon>/po/<locale>-local.po, so this invocation fails looking for po/all-local.po. Authors following the update workflow cannot refresh translations unless the instructions specify each locale separately or add actual all handling.
Useful? React with 👍 / 👎.
|
|
||
| ## Translating UI Files (Glade) | ||
|
|
||
| Gramps' addon translation tools only automatically extract and manage Python strings. If your addon uses a Glade (`.ui` / `.glade`) file for its interface, those strings will not be picked up by the standard addon translation workflow. The recommended pattern is to mark the Glade strings as translatable (so they show up for translators), then override the label at runtime from Python so they get translated through your addon's gettext domain. |
There was a problem hiding this comment.
For top-level .glade files this statement is incorrect: make.py lines 450–455 glob *.glade and append their translatable strings to template.pot with xgettext -L Glade. Telling authors that the standard workflow ignores those strings encourages redundant runtime label overrides and misrepresents what make.py init produces; only unsupported UI-file cases should be called out.
Useful? React with 👍 / 👎.
| Local invocation: | ||
|
|
||
| ```bash | ||
| ruff check --select E9,F63,F7,F82 <Addon>/ |
There was a problem hiding this comment.
Checked the repository's CI command at .github/workflows/ci.yml: it explicitly passes --exclude='*.gpr.py' because injected names in registration files are intentionally undefined to static analysis. The documented command omits that exclusion, so running it on a valid addon reports F821 for register, _, plugin-kind constants, and status constants rather than mirroring the CI gate.
Useful? React with 👍 / 👎.
| - **MUST**: `fname` points to an implementation module shipped in the same folder. | ||
| - **MUST**: the addon is physically present under the plugin path — a physical copy works on every Gramps version and OS. (Gramps 6.1+ also discovers an addon reached via a symlink, but a physical copy is the portable default.) | ||
| - **MUST NOT**: import `register`, `GRAMPLET`, `STABLE`, `_`, or any other name Gramps injects into the `.gpr.py` namespace. | ||
| - **MUST NOT**: add `__init__.py` to the addon directory itself. The plugin loader puts the addon dir on `sys.path` and imports `<Addon>.py` by name; making the addon dir a regular package disturbs that resolution and can trigger the [Mantis 12691](https://gramps-project.org/bugs/view.php?id=12691) submodule-binding trap. (See [07-testing → Why `tests/__init__.py` exists](07-testing.md#why-tests__init__py-exists).) |
There was a problem hiding this comment.
This repository already contains functioning addon roots with __init__.py, including Sqlite, Query, DynamicWeb, PlaceCleanup, and PostgreSQLEnhanced; the test runner also explicitly handles regular addon packages. Making their absence an unconditional MUST would tell maintainers to remove files needed for package-relative imports and tests, so the prohibition should be limited to the specific same-name module collision it is meant to prevent.
Useful? React with 👍 / 👎.
|
|
||
| - **MUST**: wrap every user-visible string with `_()`. | ||
| - **MUST NOT**: `import _` in `.gpr.py` — Gramps' plugin loader injects it. Implementation modules **MUST** bind it explicitly via `_ = glocale.get_addon_translator(__file__).gettext`. | ||
| - **MUST** (multi-file packages): when the addon's code is split across a nested package, bind `_` **once at the addon root** — the directory that holds `locale/`, in a root-level module (e.g. `_i18n.py`) — and import it everywhere else by **bare name** (`from _i18n import _`), **not** a `<Addon>.`-prefixed path: the addon dir is on `sys.path` and its root is **not** a package (see *Structure* → MUST NOT `__init__.py`), so a root-level module imports directly, whereas `from <Addon>.<pkg>.i18n import _` raises `'<Addon>' is not a package` at import time. `get_addon_translator(filename)` derives the catalog dir as `dirname(abspath(filename)) + "/locale"` (`gramps.gen.utils.grampslocale`), so a `get_addon_translator(__file__)` call from a nested module (e.g. `myaddon/views/tab.py`) resolves `myaddon/views/locale/`, which doesn't exist, and a non-English user silently gets the untranslated string. The flat `_ = glocale.get_addon_translator(__file__).gettext` form above is correct only because that module sits at the addon root; from a nested module, anchor the path at the root (e.g. `get_addon_translator(os.path.join(ADDON_ROOT, "_"))` — only `dirname(...)` is read, so the basename is an unused placeholder) instead of passing `__file__`. (NameSuite i18n-anchor fix, 2026-06-25.) |
There was a problem hiding this comment.
If two loaded addons follow this example and both create a top-level _i18n.py, Python caches the first one as sys.modules['_i18n']; the second addon's bare from _i18n import _ then reuses the first addon's translator and catalog. Plugin-path removal does not clear sys.modules, so the recommended generic module name creates cross-addon translation contamination; use an addon-unique top-level name or a genuinely namespaced package such as the existing NameSuite implementation.
Useful? React with 👍 / 👎.
| LOG.error("Could not parse %s", filename) | ||
| ``` | ||
|
|
||
| `__name__` for an addon resolves to the addon's `id` (e.g. `"MyAddon.myaddon"`), so the logger inherits the addon's name naturally — useful for per-logger filtering below. |
There was a problem hiding this comment.
The registration id does not determine Python's __name__: Gramps imports a flat addon's fname from the temporarily added addon directory, so DescendantSpaceTree.py, for example, logs as DescendantSpaceTree even though its registration id is descendantspacetree; nested packages use their actual package path. Consequently the suggested --debug=MyAddon.myaddon filters commonly match no logger and leave DEBUG output hidden.
Useful? React with 👍 / 👎.
| - **MUST**: open the PR body with a **`**User impact:**`** line (before Root cause), then structure it **Summary / What to look at / Root cause / Fix / Verification**, citing `path:lines` on the branch the PR targets in the Verification "Checked" line (the #106 format). | ||
| - **MUST**: when the PR modifies an addon, **call out its current maintainer** — add an `## Affected addon` section to the PR body that **@-mentions the addon's current maintainer**, a heads-up so they are *aware* of the change and don't miss it. This is awareness, not attribution. "Current maintainer" = the addon's `.gpr.py` `maintainers` field when declared, otherwise its `authors` (an addon with no separate maintainer is maintained by its original author — Doug's "original developer (or contributors)" and Nick's "current maintainer" are the same role). The `.gpr.py` records names/emails, not GitHub handles — resolve a handle best-effort from the declared email so the mention notifies, and name the person when no handle resolves. (Raised on addons-source PR #946 — Doug Blank: *"otherwise I could miss fixes to my addons"*; Nick Hall: *"mention the current maintainer if one exists."*) | ||
| - **OPTIONAL**: reference the Mantis bug in the PR body **when one exists** — a Mantis reference is optional for addons-source, since many addon fixes have no Mantis ticket (they're tracked as fork GitHub issues, or are ticketless). addons-source also does not use the `Fixes #NNNN` commit-message trailer at all — that is the core convention; see the [Core Development — Rules](https://gramps-project.org/wiki/index.php/Gramps_6.1_Wiki_Manual_-_Core_Development_-_Rules) page. | ||
| - **MUST**: keep upstream-repo cross-references out of PR text and fork issues — reference *other* upstream PRs/issues in **plain text** ("upstream PR 949"), never a GitHub URL or `owner/repo#NNN` cross-ref (it back-links/notifies that thread). The `#nnnn` Mantis reference and the PR's own target are exempt. Authoritative: `docs/INTEGRATION.md` §"No upstream-repo links". |
There was a problem hiding this comment.
A repository-wide file search and the commit tree contain no docs/INTEGRATION.md, so contributors cannot inspect the section asserted to be authoritative for this unusual MUST-level PR rule. Either include the cited policy in this repository or link to the actual source; otherwise the new normative manual relies on an unverifiable, nonexistent local document.
Useful? React with 👍 / 👎.
0f1011f to
2ec8004
Compare
…ields IsSiblingofNamedSibling crashed with HandleError whenever applied to a person with no recorded parents, because get_family_from_handle(None) raises rather than returning None. RegExpPersonal/RegExpFamily also searched mismatched Name fields (title was listed twice in the personal list, and call name was searched by the family/surname rule instead), so personal search never matched a person's call name and surname search could false-positive on an unrelated title or call name. Adds unit tests covering both regressions plus the general relationship-matching rules, and brings the addon up to date with black/mypy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…wiki page help_url pointed at Addon:AdvancedPersonFilter, a different addon name left over from this addon's origin, instead of Addon:PersonRelationshipFilter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`from PersonRelationshipFilter import (HasName, ...)` breaks when unittest loads this file as `PersonRelationshipFilter.tests.test_...`, because by then the outer `PersonRelationshipFilter` is already a namespace package in sys.modules, so the bare import looks for each name as an attribute of the package instead of importing the PersonRelationshipFilter.py submodule that defines them. Use the fully-qualified `PersonRelationshipFilter.PersonRelationshipFilter` import path, matching the pattern already used in DataEntryGramplet/tests/test_data_entry_gramplet.py. Reported by GaryGriffin in gramps-project#987. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2ec8004 to
38036e5
Compare
Some agentic coding tools look for AGENTS.md rather than CLAUDE.md; keep the two in sync so contributors using either tool see the same repo conventions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Eduard R. <eduralph@users.noreply.github.com>
- Clarify branch model: "release" means major/feature release, not patch release; note master exists but isn't currently used for addon work. - Note LANGUAGE=en_US.UTF-8 is no longer required on Gramps v6.0+, and mention checking out the matching branch in the core checkout. - Soften Black formatting guidance to reflect it's not currently required. - Note that strings already translated in Gramps core are excluded from the Weblate Addons component. - Add maintainers/maintainers_email fields to the .gpr.py example.
Per hgohel's suggestion on PR gramps-project#991 (comment 5026237564) and follow-up discussion, clarify the distinction between MSYS2 (POSIX-like shell, safe to run make.py/tests in) and native Windows cmd/PowerShell (unverified tooling) so agents know what they can and can't run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
38036e5 to
d2dabee
Compare
3 participants
Automated nightly sync from
gramps-project/addons-source@maintenance/gramps60. Generated by .github/workflows/upstream-sync.yml on the testbed.