Re: [Nano-devel] [PATCH 1/2] history: prevent overwriting of positions between multiple instances

[Benno Schulenberg]

[Please send your messages to the list, not to me.]

Op  2-11-2017 om 23:22 schreef Brand Huntsman:
> On Thu,  2 Nov 2017 20:31:31 +0100 Benno Schulenberg <address@hidden> 
> wrote:
>
> > +static struct stat stat_of_positions_file;
>
> I used *_stat for consistency with the stat in open files. typedef struct 
> openfilestruct { struct stat *current_stat; }

I like variety in names.  It helps me distinguish things.

> > +void reload_positions_if_needed(void)
>
> I like the _if_needed suffix but should it be reload_poshistory_if_needed for 
> consistency with the other poshistory functions?

Variety, again.

> > +    struct stat newstat;
>
> And again here, open files use "st" for the local variable when checking if 
> the file was modified.

I dislike short variable names, I can hardly "see" them.

> > +    if (newstat.st_mtime != stat_of_positions_file.st_mtime) {
>
> And again here, open files use "old < new".

And do_alt_speller() uses "new != old".  The latter was written by me,
the comparison in do_writeout() existed.

> Do you really want to allow an m_time older than the last loaded m_time to 
> replace the position contents?

Why not?  The file *changed*, or at least was touched -- we should
respect its contents.  Maybe the user restored a backup?  Or maybe
they overwrite the positions history every hour so that certain files
will always be opened with the cursor at a specific position?

> I would consider anything older to be corrupt and not care if its changes were
> overwritten. It is also possible the last loaded modification used a future
> timestamp and < would prevent legitimate modifications from being loaded. We
> could hack around this or just assume the timestamps aren't broken and use
> "old < new".
>
>
> > +       poshiststruct *ptr, *nextone;
>
> The posptr name I used came from the save_poshistory function, which uses 
> posprev, hence my posnext.

Yes, but all these variables with the same prefix get on my nerves.
They lack variety.

Benno