[parted-devel] History patchset

[Matt Davis]

On Feb 9, 2008 2:45 PM, Matt Davis <mattdavis9 at gmail.com> wrote:
> On Feb 8, 2008 9:01 AM, Jim Meyering <jim at meyering.net> wrote:
> > Matt Davis <mattdavis9 at gmail.com> wrote:
> >
> > > The following 3 patches implement history/undo/redo behavior for parted.
> > >
> > > This change captures potential disk modifications based on what commands a user
> > > issues.  The list of disk modifications can be displayed/queried, undone, or
> > > redone.  Eventually the changes can be commited to disk.  This prevents the user
> > > from performing unwanted modifications to disk before they are ready.  Also, the
> > > user can view the disk as if the modifications were taking place, even before
> > > the "save" their changes.
> > ...
> > > diff --git a/libparted/history.c b/libparted/history.c
> > ...
> > > +char PedHistStr[32];
> > ...
> > > +PedHistManager PedHistMgr = {0};
> > ...
> > > +void
> > > +ped_history_add (const char *name)
> > ...
> > > +void
> > > +ped_history_clear (void)
> >
> > Hi Matt,
> >
> > I see at least two global variables there.
> > The existing ones cause enough trouble, (and this is library code),
> > so please don't add new global variables.
> >
> > As I said before, you will want to adjust each of your history-manipulation
> > functions to take a pointer, probably *PedHistManager:
> >
> >     void
> >     ped_history_add (PedHistManager *h, const char *name)
>
> I started upon this idea.  While I do not have a problem with it
> per'se, it does couple the history calls more tightly with parted.
> Mostly, all of the disk modification calls (ie. "do_<action>") that
> the user interface executes, would need to also be passed an instance
> of the PedHistManager.   Similarly, the manager would have to be
> initialized in main, not _init(), and that instance would then be
> passed to the parse argument routine.  If you do not mind such tight
> coupling I will continue in that direction?  Anyone else have any
> thoughts?   I could do what gl does and have the library manage the
> history manager instance, but that would require a global for the
> library, which does not seem to be ideal.

I do apologize for the proliferation of email regarding this menial
augmentation.  But the best solution, to reduce such argument
coupling, would be to just let each device contain an instance of the
history manager.  That makes sense, to me at least, and would not
require the adding of the history manager to the do_<command>
prototypes in parted.c since the command func ptr passes a device
instance to all of these.  I can look more into this, this afternoon,
but a brief overlook seems that is the best solution.  And yes it
requires no globals either :-)

-Matt