Visidata: [defermods] move back into vdcore

Created on 7 Apr 2020  路  6Comments  路  Source: saulpw/visidata

We are going to bring back defermods and vsh into vdcore. vsh will possibly be a plugin.

Defermods is going to be integrated with:

  • DirSheet
  • SqliteSheet

Design Proposals

z^S will save to original source. ^S will save to provided input source.

  • We set defer = True in Sheets whose deferred modifications we want to track.

    • Pros: If we regret this later, it will be easier to pull out the logic into a plugin.

  • We created a DeferredSheet that deferred Sheets will co-inherit from.

Cons for defer = True design.

The plugin code works by overriding core functions and commands.

E.g.

 @Sheet.api                                                                                                                                                                              
 def newRow(self):                                                                                                                                                                       
     row = type(self)._rowtype()                                                                                                                                                         
     self._deferredAdds[self.rowid(row)] = row                                                                                                                                           
     return row  
Sheet.addCommand('d', 'delete-row', 'vd.cliprows = [(sheet, cursorRowIndex, deleteRows([cursorRow]))]; not sheet.defermods or cursorDown(1)') 



md5-3fe7b45f6f3f9514f88ed264d6aa43d3



@Sheet.api
def newRow(self):
    if self.defermods:
           [do defermods stuff]
     else:
           [do regular sheet stuff]

I am not sure if that is actually better/easier to excise than keeping all of the deferred logic in its own sheet.

improvement

Most helpful comment

I feel like we need to throw a parade for Anja! 馃巿

All 6 comments

If possible, something like this would be ideal:

@Sheet.api
def newRow(self):
    if not self.defermods:
        return newRow.__wrapped__(self)
    [do defermods stuff]

This is how plugins should extend functions in general, calling the original for the base case.

plugins/defermods.py:def newRow(self):
plugins/vtask.py:    def newRow(self, **kwargs):
vgit/diff.py:    def newRow(self, linenum, refnum, line):
visidata/cmdlog.py:    def newRow(self, **fields):
visidata/loaders/csv.py:    def newRow(self):
visidata/loaders/json.py:    def newRow(self):
visidata/loaders/png.py:    def newRow(self):
visidata/metasheets.py:    def newRow(self):
visidata/sheets.py:    def newRow(self):
visidata/sheets.py:    def newRow(self):
visidata/sheets.py:    def newRow(self):

Do I then have to figure out which ones to add it to?

I am willing to do that, of course! I feel insecure about adding it in all these places, and want to make sure it is the right thing to do.

That's a good question. Typically when posed with a list like this, each case must be considered individually and carefully. Basically, for newRow this is all about asking, could a user of this sheet (or a subclass of the sheet, if something else extends it) want to have additions be marked in 'options.color_add_pending` until they are saved?

  • plugins/defermods.py:def newRow(self): this is the plugin you're merging and will be deleted
  • plugins/vtask.py: def newRow(self, **kwargs): if it wants to have this feature, then it should consider setting defer and including the newRow logic itself. We'll want to document the exact minimum boilerplate for an extension that wants to use defermods and also needs to define its own newRow.
  • vgit/diff.py: def newRow(self, linenum, refnum, line): DifferSheet can't be saved back to the source. If it could (and that might be a very cool feature), then it would also want to use the code documented as above.
  • visidata/cmdlog.py: def newRow(self, **fields): I'm fine with cmdlogs staying as they are even if deferred is integrated. We can change it later if it is a thing.
  • visidata/loaders/csv.py: def newRow(self): does the CSV loader want this?
  • visidata/loaders/json.py: def newRow(self): or the JSON loader?
  • visidata/loaders/png.py: def newRow(self): or the PNG loader?
  • visidata/metasheets.py: def newRow(self): Do we want metasheets like the ColumnsSheet to require an explicit save step? Probably not.
  • visidata/sheets.py: def newRow(self):[Table]Sheet This is what the @Sheet.api bit does in defermods; it replaces Sheet.newRow with its own version, which should actually be calling TableSheet.newRow instead of copying its behavior. Do we want it to be allowable for all tables after all? maybe we should break out the into rowAdded or something, with the contract that an overridden newRow usually calls rowAdded (which is provided by the defermod code) unless it explicitly disallows deferred adding (i.e. the additions are made immediately and permanently).
  • visidata/sheets.py: def newRow(self): SequenceSheet This is somewhere between TableSheet and CSVSheet. If they're the same then this should follow suit.
  • visidata/sheets.py: def newRow(self): IndexSheet This is a superclass for sheets sheets of other sources (where each member corresponds to a sheet). I would say it should do whatever TableSheet does, and let its subclasses make the distinction otherwise.

Basically we need an interface for components and plugins, both internal and external, to follow. Ideally in my mind, if we're going to integrate this, any Sheet that has .defer set to True would have all the deferring behavior. Then it could be enabled/disabled at run-time on any Sheet.

Internally though it seems like we're going to need to do something special for newRow and other places that defermods touches.

One possible interface is a base-class, like DeferredSheet, which integrates with the universal TableSheet directly, and localizes the defer interface; but anything that does not explicitly inherit from DeferredSheet can't benefit from this feature without rearchitecting or refactoring DeferredSheet again. It definitely can't be enabled or disabled for any sheet. Moreover, if a sheet wants to inherit from some other sheet which also inherits from TableSheet, now we have the dreaded diamond multiple inheritance (which can be as expensive as it sounds).

A variant on the above, which at least solves the diamond problem, is a "mix-in" baseclass, which has the various functions overrided, but does not inherit from TableSheet itself. Then the defer code is localized to interactions with this parent class only, which is nice, but still it can't be enabled or disabled on a live sheet.

Is there another mechanism besides setting defer=True on the class/object, which would allow to to have a better interface for plugins?

The rowAdded design currently seems like the best option to me. This would suggest that markDeleted should be renamed to rowDeleted. (for some reason markAdded doesn't work for me). That symmetry is nice. But it suggests we should have a rowChanged and that gets a little more complicated since it takes a column too. What do you think?

More notes from saul:

rowAdded would be a callback of sorts. something that needs/wants to be notified on some action

This has been done. C:

I feel like we need to throw a parade for Anja! 馃巿

Was this page helpful?
0 / 5 - 0 ratings

Related issues

khughitt picture khughitt  路  12Comments

geekscrapy picture geekscrapy  路  12Comments

suntzuisafterU picture suntzuisafterU  路  11Comments

aborruso picture aborruso  路  12Comments

cbrueffer picture cbrueffer  路  12Comments