Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Paul,

On Mon, 14 Mar 2016, Stefan Beller wrote:

> On Sat, Mar 12, 2016 at 2:46 AM, Paul Tan <pyokagan@gmail.com> wrote:
> > These functions can be used for loading and saving common rebase options
> > into a state directory.
> >
> > Signed-off-by: Paul Tan <pyokagan@gmail.com>
> > ---
> >  rebase-common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  rebase-common.h |  4 ++++
> >  2 files changed, 73 insertions(+)
> >
> > diff --git a/rebase-common.c b/rebase-common.c
> > index 5a49ac4..1835f08 100644
> > --- a/rebase-common.c
> > +++ b/rebase-common.c
> > @@ -26,3 +26,72 @@ void rebase_options_swap(struct rebase_options *dst, struct rebase_options *src)
> >         *dst = *src;
> >         *src = tmp;
> >  }
> > +
> > +static int state_file_exists(const char *dir, const char *file)
> > +{
> > +       return file_exists(mkpath("%s/%s", dir, file));
> > +}
> 
> How is this specific to the state file? All it does is create the
> leading directory
> if it doesn't exist? (So I'd expect file_exists(concat(dir, file)) to
> have the same
> result without actually creating the directory if it doesn't exist as
> a side effect?
> 
> If the dir doesn't exist it can be created in rebase_options_load explicitly?

In addition I want to point out that sequencer's replay_opts seem to be at
least related, but the patch shares none of its code with the sequencer.
Let's avoid that.

In other words, let's try to add as little code as possible when we can
enhance existing code.

Ciao,
Dscho