serokell / coffer

Multi-backend password store with multiple frontends
4 stars 2 forks source link

[#52] Show qualified paths in `coffer` messages #58

Closed DK318 closed 2 years ago

DK318 commented 2 years ago

Description

Problem

coffer messages don't show backend name in paths. Sometimes we need it. For e.g.

coffer copy 'b1#/a' 'b2#/a'
[SUCCESS] Copied '/a/b' to '/a/b'.

It looks very odd.

Solution

Supported qualified paths in coffer messages. Now example above would print next

coffer copy 'b1#/a' 'b2#/a'
[SUCCESS] Copied 'b1#/a/b' to 'b2#/a/b'.

Related issue(s)

dcastro commented 2 years ago

I'm starting to think Entry should have a QualifiedPath EntryPath instead of an EntryPath.

But this is outside the scope of this PR. I'll have a deeper look at this later and re-valuate where and how we should use QualifiedPath.

LGTM :+1:

MagicRB commented 2 years ago

Regarding the code itself, LGTM. Architecturally:

  1. QualifiedPath is a weird a name, especially when specialized as QualifiedPath Entry. Imo we should rename it and maaybe add a class constraint just so the types are a bit tighter and represent more what we meant
  2. I don't think Entry should take on a QualifiedPath EntryPath or such field. Two entries from two backends should be equivalent, once an entry is read, its not bound to any one backend, it just exists. Adding more data on top of it with QualifiedPath or similar is imo the right way to go