Matthias Debus 2 years ago · edit

Hi Sean,

thanks for putting work in qcard! I wrote it out of the same need - replacing khard which wasn't what I wanted.

I removed the binary from git, so you are welcome to send patches.

There are two significant bundles of work. The first adds local vdir support. AddressBooks can be defined as pointing to a local directory of .ics files -- this is how the local caches of tools like vdirsyncer look. qcard can act entirely offline with a vdir. This fits great. Please send a patch.

The secord commit refactors the code: it removes dead code, localizes variables that don't need to be global (and reduces global variables in general), renames variables following Go naming conventions (type thingStruct struct is discouraged as being redundant), replaces functions with methods where appropriate, generalizes some specialized functions, and generally cleans things up. qcal and qcard are my first Go projects, so some refactoring from a more skilled person than me is more than welcome. :)

I'm pretty busy at the moment so I don't have time for many new features in qcard, but I'll be happy to merge your patches.

Regarding your German: Also, ich verstehe dein Deutsch, aber es klingt lustig. :)

Greets, Matthias

On 11.10.22 13:12, ~ser wrote:

I've been doing some work on qcard in my fork:

https://git.sr.ht/~ser/qcard

in the hopes of eventually replacing khard, which isn't too machine-friendly (for scripting). However, you checked in the compiled binary at one point, so all of the patches, regardless how small, are over 10MB and can't be submitted through email.

There are two significant bundles of work. The first adds local vdir support. AddressBooks can be defined as pointing to a local directory of .ics files -- this is how the local caches of tools like vdirsyncer look. qcard can act entirely offline with a vdir.

The secord commit refactors the code: it removes dead code, localizes variables that don't need to be global (and reduces global variables in general), renames variables following Go naming conventions (type thingStruct struct is discouraged as being redundant), replaces functions with methods where appropriate, generalizes some specialized functions, and generally cleans things up.

My next two areas of focus will be writing some badly-needed unit tests, and I may look at the bespoke ICAL parsing code and see if using an ICAL library that implements RFC 6350 more completely.

As I mentioned, I tried to submit a patch, but that qcard binary defeated my mail server. I think if we can delete that and get sync'd, I can use the standard patch process.

I'd like to collaborate, rather than hard fork; please let me know if you're interested in doing that. It would also be useful to know if/what you're working on and the direction you're going -- I have some specific needs I want to accomplish, for my own purposes, and would be focusing on those.

If you want to merge my changes, you'll have to remote add my repo and merge them on your desktop; after that, the qcard binary will be gone and I can submit patches. There are only the two commits, so at least the review won't be horrible.

Ich kann gut Deutsch lesen, habe aber vor 20 Jahren durch Eintauchen gelernt und wurde dar??ber informiert, da?? mein geschriebenes Deutsch fast unverstandbar ist. Also, ich schreibe lieber English.

Cheers,

-- View on the web: https://todo.sr.ht/~psic4t/qcard/1

-- message me on Matrix: @psic4t:data.haus

Matthias Debus referenced this from #1 2 years ago

~ser 2 years ago

On Tue Oct 11, 2022 at 12:00 PM CDT, Matthias Debus wrote:

qcal and qcard are my first Go projects, so some refactoring from a more skilled person than me is more than welcome. :)

Yeah, I just saw qcal in your list; I'll be looking at that, too.

I'm pretty busy at the moment so I don't have time for many new features in qcard, but I'll be happy to merge your patches.

Same here; this will be the fourth project I've picked up sending patches for; I'm quickly running out of bandwidth. That's another reason why I don't want to do a hard fork.

With qcard, I would like it to be able to replace vdirsyncer. I think it's only a few more lines of code, now that it supports both ical and local ics. I don't know if that counts as a "local cache."

My patches bounced from the todo.sr.ht email; the mail server said they were too big, over 16k. That doesn't seem like a large patch to me, but I may be using git send-email wrong. Anyway, I sent them to the public mailing list, which was probably bad form, but there you go.

--- SER
Sean E. Russell
Age: age195vpft7nzsy83medxagqqsge0lrcuf9txe3z2znlu2wsk69cdu4sx8nfvp
Minisign: https://ser1.net/.well-known/minisign.pub
GPG key: https://ser1.net/.well-known/pgp.asc
Matrix: @ser:matrix.org

~ser 28 days ago

Matthias -- I've pushed a new set of changes, and I think it's time to do some coordination because I think we may be diverging.

Most recently, I noticed a number of parsing issues which led me to ultimately replacing all of the bespoke vcf parsing code with a library. The short version is that with -emailonly I was getting 62% of my 955 vcards parsed with the bespoke regexp parser, and with the library I get 70% -- incidentally, exactly the same list of successful email extractions as with khard. Also, with the regexps being re-created with every card parsed, that's 32 regular expressions being re-created with every card, which was really impacting performance. With the library, I am getting this:

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
khard email --parsable --remove-first-line	511.7 ± 6.3	504.5	520.9	23.22 ± 0.88
qcard-old -emailonly	520.9 ± 4.6	515.3	528.7	23.64 ± 0.87
qcard-new -emailonly	22.0 ± 0.8	20.3	24.3	1.00

A 24x performance improvement: 132 runs vs 10 runs with the regexps. I picked the emersion/go-vcard library as, of the two libraries I benchmarked, it was both the fastest and also the only one that parsed all 955 vcfs.

I will be trying a rebase, but I see the changes you've been making and I'm sure there are going to be significant conflicts, as you're in making changes to code that I've completely erased with the library. I'll give that a try and let you know, but what are your thoughts about moving forward?

I do not expect to be making more large changes. I got to working on the parser because I was regularly missing email completions that khard was providing. If we can get past this merge, I think I'll be done for a while. I really don't want to put in hours trying to resolve conflicts if you're going to object to the use of a vcard library, or some other aspect, though -- would you mind reviewing my fork and seeing if there's anything that concerns you? In the meantime, I'll try a rebase and see just how awful it's going to be.

Tschuß

~psic4t 28 days ago

Hello Sean,

some thoughts about this:

• I have a hard time figuring out why you are doing certain things in your fork. It would be much easier for me if you just submit patches for a single feature/refactor.
• Actually one of the goals of all my tools is not to be dependent on external libraries, so I would prefer patches to the existing parser.

Can you submit a single (small) patch to the existing codebase first? This should work over todo.sr.ht.

Tschüß!

~ser 27 days ago

Ok! That certainly simplifies things, thank you.

I think the best thing for me would be to just hard-fork into a different project. I understand -- and sympathize with! -- the desire to not pull in other dependencies. I did so only reluctantly for three reasons:

• The effort to refactor the current code to remove the contributors to the performance issues is significant. Not hard, but tedious. The parser is recompiling each of the 32 regular expressions every time it reads a new vcard. The refactor solution would be to extract and pre-compile all of those compilations out into an init() (or main(), at the very least).
• Using regular expressions for the parsing makes the parser a little easier to write, but is an inefficient way of parsing.
• RFC6350 -- the vcard specification -- is not trivial to implement with regular expressions. It allows, for example, TYPES to be expressed in any permutation of values -- these make for complex and difficult to debug regexps, but also in some cases are literally impossible using the built-in RE2-based Go engine, which does not provide look-arounds. Adopting a PCRE or more capable (but non-O(1) time) engine would require an external dependency.

Ultimately, I think may be impossible to make the current parser correct without bringing in an external regexp parser, and in any case would still result in a relatively slow implementation. I did initially try to fix the regexps, but dealing with the permutability of value order without look-around in RE2 made the regexps monstrous, and I had the feeling that I was trying to fix a mountain of a problem by chipping away at rocks with a little hammer.

For me, performance is critical as I'm using this to drive the auto-complete function in my mail client. It's also important that the tool be able to correctly parse all of the cards. The current implementation of qcard is no faster than khard and produces less correct results, often omitting valid results because of parsing errors.

I've been watching at the commits on your project, and I think we have different enough styles and objectives to warrant a hard-fork for entirely objective reasons. I'll keep the accreditation, change the project name on my end, and wish you the best of luck!

Administrative stuff: I honestly would have preferred to not start maintaining another package -- merely because I don't have time to invest in them all -- but I think it's the best option in this case. I have a workflow that allows me to package and publish to Arch AUR (my main distro) and Alpine, and I'm in the process of adding Void. I also generally build binaries for RPM and DPKG and make these available on my file server. You will probably see my fork show up in AUR; does brief project description -- for the repositories -- look amenable?

"NEWPRJNAME is a CLI address book application for CardDAV and VCF directories. It is a hard fork of qcard with different priorities."

In the README, I'll credit and reference qcard, and describe the differences as I see them. While I tend to prefer BSD 3-Clause, I'll leave the GPL-3 license.

Any requests for the README?

And -- sincerely -- thanks. I needed something better than khard, and qcard has been a good basis.