[virt-tools-list] [libosinfo 1/4] Add InstallScript:expected-path-format

[Zeeshan Ali (Khattak)]

On Mon, Nov 12, 2012 at 4:06 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Mon, Nov 12, 2012 at 03:55:56PM +0100, Zeeshan Ali (Khattak) wrote:
>> On Mon, Nov 12, 2012 at 11:37 AM, Christophe Fergeau
>> <cfergeau at redhat.com> wrote:
>> > On Sun, Nov 11, 2012 at 08:14:55PM +0100, Zeeshan Ali (Khattak) wrote:
>> >> From: "Zeeshan Ali (Khattak)" <zeeshanak at gnome.org>
>> >>
>> >> Inform the applications in which format the paths are expected by the
>> >> installer.
>> >> ---
>> >>  data/install-scripts/windows-cmd.xml      |  2 +-
>> >>  data/install-scripts/windows-reg.xml      |  2 +-
>> >>  data/install-scripts/windows-sif.xml      |  2 +-
>> >>  data/install-scripts/windows-unattend.xml |  4 ++--
>> >>  osinfo/libosinfo.syms                     |  4 ++++
>> >>  osinfo/osinfo_install_script.c            | 28 ++++++++++++++++++++++++++++
>> >>  osinfo/osinfo_install_script.h            | 22 +++++++++++++++++-----
>> >>  osinfo/osinfo_loader.c                    |  9 +++++++++
>> >>  8 files changed, 63 insertions(+), 10 deletions(-)
>> >>
>> >> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
>> >> index 2292aac..2133618 100644
>> >> --- a/osinfo/osinfo_install_script.c
>> >> +++ b/osinfo/osinfo_install_script.c
>> >> @@ -59,6 +59,7 @@ enum {
>> >>      PROP_TEMPLATE_DATA,
>> >>      PROP_PROFILE,
>> >>      PROP_PRODUCT_KEY_FORMAT,
>> >> +    PROP_EXPECTED_PATH_FORMAT,
>> >
>> > Not sure about the 'expected' part of the property name "path-format"
>> > should be enough?
>>
>> IMO its much more descriptive yet short enough. Also, its more
>> consistent with the existing _get_expected_filename() method.
>
> And is not consistent with the PROP_PRODUCT_KEY_FORMAT right above,
> I don't
> really see what the 'expected' brings.

Now that I think of it, the 'expected-' prefix was added in the
_get_expected_filename() to differentiate it from _get_filename(). So
I'll remove it from this and other new API I proposed in this series.

>> >> +/**
>> >> + * OsinfoPathFormat:
>> >> + * OSINFO_PATH_FORMAT_UNIX: Unix/Linux path format, e.g /home/foo/bar.txt
>> >> + * OSINFO_PATH_FORMAT_DOS: DOS/Windows path format, e.g \My Documents\bar.txt
>> >
>> > Shouldn't this have a drive name as well? C:\My Documents\bar.txt
>>
>> As you'll see in the following patches, we'll have different params
>> for disk/drive and path so that such API are as consistent as possible
>> for different OSs.
>
> Ah ok, might be worth explicitly spelling out that the drive name should
> not be there as this is not your typical Windows path.

I do spell out in the docs of the appropriate getters:

 "Also note that in case of #OSINFO_PATH_FORMAT_DOS, the drive/disk letter
 and the leading ':' must not be included in the path."

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124