insanum / gcalcli

Google Calendar Command Line Interface
MIT License
3.33k stars 314 forks source link

Refactor to use more helpers from `details` #773

Open dbarnett opened 1 month ago

dbarnett commented 1 month ago

As follow-up from #550, the code could use some cleanup to make better use fo common details helpers:

@michaelmhoffman

michaelmhoffman commented 1 week ago

In gcalcli.gcal there are a number of functions that are long repeats of conditional statements converting various details of events between the Google API format and a human-readable text format or back. It is difficult to comprehend these functions because they are so long, and since gcalcli provides several different commands for both reading and writing to the Google Calendar, duplication and subsequent inconsistency is inevitable.

There were even more of these, before I added the gcalcli.details architecture, which instead provides a subclass of gcalcli.details.Handler for each detail. The subclasses are not meant to be instantiated. They provide only organization and an inheritance hierarchy.

Each Handler has a .get(event) classmethod that returns a simple string representation for columnar output (agenda --tsv) and a .patch(cal, event, fieldname, value) classmethod that converts the string representation back into something that works with everything else.

I refactored gcalcli.gcal._tsv() to use the details architecture, and gcalcli.gcal.AgendaUpdate() has used it from the start.

This greatly simplified the code for _tsv():

def _tsv(self, start_datetime, event_list):
    keys = set(self.details.keys())
    keys.update(DETAILS_DEFAULT)

    handlers = [handler
                for key, handler in HANDLERS.items()
                if key in keys]

    header_row = chain.from_iterable(handler.fieldnames
                                     for handler in handlers)
    print(*header_row, sep='\t')

    for event in event_list:
        if self.options['ignore_started'] and (event['s'] < self.now):
            continue
        if self.options['ignore_declined'] and self._DeclinedEvent(event):
            continue

        row = []
        for handler in handlers:
            row.extend(handler.get(event))

        output = ('\t'.join(row)).replace('\n', r'\n')
        print(output)

Handlers are looked up in the details.HANDLERS dict from the details supplied on the command line. Then the function dispatches to the Handler classes to do all the magic. Each Handler provides its own name for the header of the TSV, and then the textual representation for each event.

The code for AgendaUpdate() is also simple:

def AgendaUpdate(self, file=sys.stdin):
    reader = DictReader(file, dialect=excel_tab)

    if len(self.cals) != 1:
        raise GcalcliError('Must specify a single calendar.')

    cal = self.cals[0]

    for row in reader:
        action = row.get('action', ACTION_DEFAULT)
        if action not in ACTIONS:
            raise GcalcliError('Action "{}" not supported.'.format(action))

        getattr(actions, action)(row, cal, self)

Each row dispatches to one of the gcalcli.actions functions (patch, insert, delete, or ignore). patch() and insert(), in turn, call Handler.patch() for each detail, ensuring processing of text to programmatic interface is provided consistently and without duplication.

michaelmhoffman commented 1 week ago

My suggestion for priority of refactoring is:

  1. Refactor other cases where events are edited or patched (edit, quick, add) event to use Handler.patch() instead. This is the most compelling case for refactoring because a lot of the details in these subcommands either do or should have consistent treatment for writing with other parts of gcalcli, and I think there is the greatest chance of error or consistency by not doing this.
  2. Add format() classmethods to Handlers to replace the implementation in methods that have a more formatted output (like agenda without --tsv). Lower priority but I think would still be helpful in making the code more maintainable.
  3. Add classmethods to patch from ICS format instead of the agenda round-trip free text used by .patch() elsewhere, replacing the guts of gcalcli.gcal.ImportICS(). Lowest priority as this code is pretty self-contained.