On Sun, 05 Jul 2026 at 12:46:14 +0200, Oswald Buddenhagen wrote:
> > The patch had two goals:
> > 1) Un-intertwine the current two implementations.
> >
> it doesn't achieve that to any appreciable degree. the cyrus implementation
> still hard-codes the login special case,
Yes, both implementations (Cyrus & simple) share the LOGIN logic. But I
fail to see how having some logic in common means the goal overall is
not met.
> and presumably that would be
> duplicated in the gsasl impl.
FWIW, I have made an implementation with GSASL and I've found the
library can handle LOGIN for us, making its use much simpler compared to
Cyrus SASL.
> > 2) Add the infrastructure to allow a third implementation.
> >
> as i said previously, just add the gsasl impl. with ifdefs.
Okay, I can do that. But I _really_ do not think it would be a good
idea to continuously keep the implementations weakly separated with
#ifdefs scattered across a file. It requires WAY more mental work to
parse and is hence harder to maintain for no discernible gain. And it
strongly encourages spaghetti code due to the lack of clear structure
and logical boundaries.
> > > then it also rewrites (and bloats) the mechanism selection, for no
> > > apparent reason.
> >
> > [...] This is a benefit of such an interface: it allows more flexibility
> > with its use.
> >
> > [...] If mbsync sometime decides to add support for [...]
> >
> as i already told you twice, i do not want code that exists "just in case".
I see; my apologies for not getting the message. I can try to retain
the original mechanism filtering logic inline if that's desired.
Though would you be open to changing the fixed-size buffer to a dynamic
one? It's much better to be dynamic when the buffer size is unknown
beforehand, rather than choosing an arbitrary fixed size and hoping it
will be enough.
> the second complaint is about the implementation itself: you're joining the
> lists early, making the subsequent manipulations require finicky string
> operations. that's counter-productive.
That's a fair criticism. The string manipulation was a consequence of
generalising the selection, so it shouldn't be present if we keep it
hardcoded as the intersection of exactly two lists.
Take care,
Seth McDonald.
--
E9D1 26A5 F0D4 9DF7 792B C2E2 B4BF 4530 D39B 2D51
_______________________________________________
isync-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/isync-devel