[parted-devel] History with undo redo

[Otavio Salvador]

Matt Davis <mattdavis9 at gmail.com> writes:

> --- a/include/parted/disk.h
> +++ b/include/parted/disk.h
> @@ -261,10 +261,13 @@ extern PedDisk* ped_disk_new_fresh (PedDevice* dev,
>  extern PedDisk* ped_disk_duplicate (const PedDisk* old_disk);
>  extern void ped_disk_destroy (PedDisk* disk);
>  extern int ped_disk_commit (PedDisk* disk);
> -extern int ped_disk_commit_to_dev (PedDisk* disk);
>  extern int ped_disk_commit_to_os (PedDisk* disk);
>  extern int ped_disk_check (const PedDisk* disk);
>  extern void ped_disk_print (const PedDisk* disk);
> +/* Located in parted/history.h
> + * extern void ped_disk_commit_to_history (const PedDisk* disk);
> + */
> +extern int ped_disk_commit_to_dev (PedDisk* disk);
>  
>  extern int ped_disk_get_primary_partition_count (const PedDisk* disk);
>  extern int ped_disk_get_last_partition_num (const PedDisk* disk);

I prefer ped_disk_commit_to_history on disk.h like others...

> --- a/libparted/disk.c
> +++ b/libparted/disk.c
> @@ -510,6 +510,17 @@ ped_disk_commit (PedDisk* disk)
>  }
>  
>  /**
> + * Adds the changes to disk to the command history.
> + * This allows the commands to have an undo/redo effect
> + * without actually performing any (possibly unwanted)
> + * disk actions.
> + * 
> + * This is actually defined in parted/history.c
> +void 
> +ped_disk_commit_to_history(const PedDisk *disk)
> +*/

Same to this.

> --- a/parted/Makefile.am
> +++ b/parted/Makefile.am
> @@ -9,6 +9,8 @@ parted_SOURCES = command.c	\
>  		 strlist.h	\
>  		 ui.c		\
>  		 ui.h		\
> +         history.c  \
> +         history.h  \
>  		 table.c	\
>  		 table.h

Wrongly indented

> --- /dev/null
> +++ b/parted/history.c
> @@ -0,0 +1,193 @@
...
> +void ped_disk_commit_to_history(const PedDisk *disk)
> +{
> +    PedHistMgr.end->disk = ped_disk_duplicate(disk);
> +}

Please, move it to disk.{c,h}

> +static void history_pr(PedHistObj *obj)
> +{
> +    /* Only print disk changes */
> +    if (!obj->disk)
> +      return;
> +
> +    printf("[History %d]\t", obj->id);
> +    str_list_print_wrap(obj->cmd.names, screen_width(), 8, 8);
> +    if (obj->ignore)
> +      printf(" (UNDONE)");
> +    fputs("\n", stdout);
> +}
> +
> +
> +/* Print range (or specific if start == end) */
> +void history_print(int start, int end)
> +{
> +    int         i;
> +    PedHistObj *ii;
> +
> +    for (i=0, ii=PedHistMgr.begin;
> +         ii && i>=start && i<end;
> +         ii=ii->next, i++)
> +    {
> +        history_pr(ii);
> +    }
> +}

history_pr could be written inside of history_print. I see no reason
for the spliting.

> --- /dev/null
> +++ b/parted/history.h
> +/* Remember changes 
> + * Note: This is not defined in disk.h
> + *       This preserves encapsulation between
> + *       PedCommands and libparted
> + */
> +extern void ped_disk_commit_to_history(const PedDisk *disk);

Can you elaborate this comment?

One thing I do think is important here is to have tests for it. It's a
quite big change and would be good to be sure we don't keep stupid
bugs on it.

-- 
        O T A V I O    S A L V A D O R
---------------------------------------------
 E-mail: otavio at debian.org      UIN: 5906116
 GNU/Linux User: 239058     GPG ID: 49A5F855
 Home Page: http://otavio.ossystems.com.br
---------------------------------------------
"Microsoft sells you Windows ... Linux gives
 you the whole house."