Repository navigation
Replies: 1 comment
|
All four tracks are on
The open questions, as decided1. The ten dead methods — delete. Verified against the code first: they have exactly one caller each, the façade delegation, and nothing else. The six flags became a Wiring them up instead would have realised the original intent, but it meant changing live UI behaviour for S3/SSH/archive panes inside the change that publishes the interface. That seemed like the wrong two things to do at once. 2. A set of names, per the post's own lean. 3. The sample lives in 4. Confirmed: URI string in, Two changes to the plan③ landed before ②. Writing
The measurement
Three things found on the way
The registry had a shadowing bug, found by ①b's Dogfooding #451's own sample found three more. The template carried the class and the Worse, nothing reported it. Jump to Path's (The third was mine: the docs said Jump to Path is Ctrl-J. It is Shift-J.) Not in it
|
Uh oh!
There was an error while loading. Please reload this page.
Prompted by #413: someone wants to browse the Windows registry in a pane — read-only, with the actual editing handed off to regedit — and asked whether
config.pycan define a virtual folder the way cfiler does. They then had an LLM produce a monkeypatch, which works, and which found every place I would have had to fix anyway.Scheme registration is the visible ask, and it is the cheap half. The half that decides whether this is usable is what a third party has to write once they have registered, and what I am promising not to break afterwards. Same shape as #396 and #378: the mechanism underneath is the thing worth getting right.
1. How it works today
Path(xefm/path.py) is a façade that delegates to aPathImpl, chosen in_create_implementation(path.py:1086-1121) by an if-chain on the URI prefix. The contract there is already uniform and already right: a URI string goes in, aPathImplcomes out —ArchivePathImpl(path_str),S3PathImpl(path_str),SSHPathImpl(path_str). Turning that into a table lookup is not a redesign.The scheme list itself is hardcoded in three places, and #413's monkeypatch found all three:
PathlibPathXeFMApp._REMOTE_SCHEMES, which Jump to Path resolves against_REMOTE_SCHEMES, forDRIVE_LOCATIONSPatch only the first and Jump to Path treats
reg://…as a path relative to the current directory and reports "Path does not exist". That report is accurate.Two smaller things fall out of looking.
scp://andftp://sit in all three lists with no branch in_create_implementation, so they quietly resolve toLocalPathImpl. And the config side needs no new machinery:xefm/user_api.pyalready loads, validates and reloadsACTIONS/EVENT_HOOKS/SORT_KEYS/FILTERSdeclaratively, warning per entry and never failing the whole config.PATH_SCHEMESis one more table in that same pass.2. Registration alone gives #413 nothing
PathImplhas 61 methods. 60 of them are abstract;listdir_attrs(path.py:267) is the only concrete one, and it is concrete only because it is written in terms ofiterdir.The measurement already exists in the repo.
test/test_mock_storage_extensibility.pyexists to prove that a new storage type needs no UI changes, and to prove it, it writesMockPathImpl— 384 lines, 61 methods, for a backend that keeps three files in a dict. That is what I would be handing someone who wants to list registry keys.3. What can be defaulted, and on what grounds
Two different justifications get mixed together here, and separating them decides the class layout.
parentandjoinpathacross the three remote impls reduce to the same operation:s3://bucket/key/pathssh://host/abs/patharchive://<file>#internal/pathSplit on
/, drop or append a segment, re-join with the prefix. The differences are the prefix spelling, the trailing-slash convention, and behaviour at the root — where archive is genuinely different, because the parent of an archive's root leaves the archive. Two hooks (_root_prefix(),_with_key()) cover the first two, and the third is exactly what an override is for.By group, then:
nameparentpartsjoinpathwith_*relative_toglob__eq__…mkdirunlinkrenamewrite_byteschmod…supports_*get_search_strategyget_display_*…existsis_diriterdirstatopen…Which argues for two layers rather than one class with everything filled in:
Keeping them apart matters because
UriPathImplis not read-only-specific. s3.py and ssh.py carry roughly 290 lines between them implementing those same 17 arithmetic methods twice — a debtUriPathImplcan eventually absorb, and could not if the same class had also decided that writing raises.De-abstracting
PathImplitself is the one-class version of this, and it costs the check that currently catches an internal impl forgetting a method at import time. The ABC should stay strict; the published base is where leniency belongs.4. Is
PathImplstable enough to publish?Signatures, yes — not one has been changed or removed, ever. Growth, no.
PathImpladded (thensrc/tfm_path.py)listdir_attrs48 of the 61 are names
pathlib.Pathalso has, which is why they do not move — that half is pinned to the stdlib rather than to my judgement. 13 are XeFM's own, and every one of the nine additions above landed in those 13.listdir_attrsarrived because the async listing work wanted a per-backend hook. Nothing about that pattern has finished.The failure mode is not a signature change. It is a new
@abstractmethod: the moment one lands, every external subclass raisesTypeErrorat import, and the user's entire config stops loading. Six quiet weeks is not a track record.So the published extension point cannot be
PathImpl. If it isReadOnlyPathImpl, a future method arrives carrying a default and outside code never notices. The intermediate class is the compatibility buffer; the ergonomics are a side effect. Worth a test that fails when a new abstract method has no default in the published base, so the rule outlives my remembering it.5. The capability methods, and the ten that nothing calls
These are compile-time constants:
supports_write_operationssupports_directory_renamesupports_file_editingsupports_streaming_readrequires_extraction_for_readingshould_cache_for_searchget_search_strategy'streaming''buffered''buffered''extracted'get_display_prefix'''S3: ''SSH: ''ARCHIVE: 'is_remoteOnly
ArchivePathImpl.is_remote(archive.py:2312) varies per instance, by asking the archive file's own path.And then the part I did not expect.
supports_directory_rename,supports_file_editing,supports_write_operations,requires_extraction_for_reading,supports_streaming_read,should_cache_for_search,get_search_strategy,get_display_prefix,get_display_titleandget_extended_metadatahave no callers in the application at all. One abstract declaration, four implementations, one façade delegation at path.py:1379-1418, some tests — and nothing anywhere that reads the answer. The file list stats directly (file_list_manager.py:537).doc/dev/PATH_POLYMORPHISM_SYSTEM.mdstill shows TextViewer / InfoDialog / SearchDialog consuming them; that diagram is stale. What is live isis_remote,get_schemeandlistdir_attrs.Publishing the interface as it stands would mean asking an outside implementer ten questions and discarding every answer.
So they collapse into a declaration instead of ten methods:
Six flags become a set, three become plain values, and three stay methods because they compute something per instance (
is_remote,get_display_title,get_extended_metadata). The set form is forward-compatible by construction — a capability added later is simply undeclared by classes written today — which takes this group out of §4's hazard entirely. Unknown names warn and are ignored, exactly asEVENT_HOOKSalready does.6. Four tracks, and the order
UriPathImpl/ReadOnlyPathImplMockPathImpl's 384 lines become ~10 methodsPathImpl+ the four implsscp:///ftp://PATH_SCHEMESconfig surfaceNo two of them edit the same region, so they are separate PRs that will not conflict. Alpha needs all four plus a sample and docs, but each lands on its own merit before then.
user_api.API_VERSIONis already0for this exact reason, and the same Preview framing covers this.② first is deliberate: publishing
PATH_SCHEMESbefore there is a class worth inheriting means advertising a registration point for something nobody can write. ③ before ①b for the same reason — changing what a capability means after publication is the breaking change this whole post exists to avoid.7. Open questions
EVENT_HOOKS), or a frozen dataclass (typos caught at construction, fields discoverable)?xefm/tools/is for end-user external programs, which this is not.PathImplout", with parsing inside the class — that is what keeps ② testable with ①a nowhere in sight.All reactions