Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to use more helpers from details #773

Open
dbarnett opened this issue Sep 25, 2024 · 2 comments
Open

Refactor to use more helpers from details #773

dbarnett opened this issue Sep 25, 2024 · 2 comments
Labels
development Internal issues that mostly affect maintainers vs. end users

Comments

@dbarnett
Copy link
Collaborator

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

  • Refactoring other uses of patch to call details.Handler subclasses
  • Add Detail.format() handlers that could be used for gcalcli.gcal.GoogleCalendarInterface._PrintEvent().

@michaelmhoffman

@dbarnett dbarnett added the development Internal issues that mostly affect maintainers vs. end users label Sep 25, 2024
@michaelmhoffman
Copy link
Collaborator

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Internal issues that mostly affect maintainers vs. end users
Projects
None yet
Development

No branches or pull requests

2 participants