An unofficial terminal client for Tangled, optimized for humans and agents. tgcli.wisp.place

cli atproto go tangled

329

Fork 10

repo edit: support cosmetic repository names #55

Closed

secluded.site 3mo ago

targeting master

Closed opened by and assigned to

secluded.site 3mo ago targeting master

Hi, thank you for creating tg! These are a few changes leading up to support for managing cosmetic names.

Add tg repo edit --name for setting or clearing a repository’s cosmetic name
Treat sh.tangled.repo.name as display data rather than repository identity
Resolve renamed repositories using newest-first record order and repoDid
Guard record edits with compare-and-swap to avoid overwriting concurrent updates
Preserve unknown repository fields while validating the modelled schema
Return the complete updated repository state after an edit
Empty names and descriptions remove their optional record fields instead of writing empty values

Happy to make any tweaks/additions/removals! 😃

This pull request has no versions submitted yet.

Comments

secluded.site 3mo ago

Hrm this is my first tangled PR and I'm very sad they do the GH/GL/et all thing of prioritising the likely-monstrous diff over letting you review smaller sets of changes in chunks by going through nicely-prepared series :c Most everything else has been pretty nice though.

aly.codes 3mo ago

hey thanks!

internal/app/repos.go:52

so this is actually changing some recent changes on f33c8db that were made in consult with the tangled community--the canonical repo is actually supposed to be the one where the record rkey matches the name value. this is meant to allow aliases (and in the future, more metadata will be removed to the knot). this strikes me as weird and troublesome (imo they should be TIDs for stable records), but it's how tangled.org/core does it, so we should match it.

downstream of this is that the edit functionality added here breaks this entire logic, and could lead to unexpected behavior. I know it would be a somewhat rare situation where the latest is not the canonical, but it's still a correctness issue imo. when we're editing name as a field itself, this could lead to unexpected behavior and poor interactions with the tangled appview

I think the swapRecord logic here is sound, though, and could be merged independently. It could introduce some unexpected failures but the right thing to do is to fail loudly in those situations, so I'm all for it.

secluded.site 3mo ago

Sure, that makes sense!

I think the swapRecord logic here is sound, though, and could be merged independently.

would you prefer that in a separate PR after closing this one, or as another round on this?

aly.codes 3mo ago

I think a separate one would be preferable!