On Thu, Aug 9, 2012 at 6:14 AM, Fabiano Fidêncio <fabiano@xxxxxxxxxxxx> wrote:
> On Wed, Aug 8, 2012 at 2:39 PM, Zeeshan Ali (Khattak)
> <zeeshanak@xxxxxxxxx> wrote:
>> On Thu, Aug 2, 2012 at 7:39 PM, Fabiano Fidêncio <fabiano@xxxxxxxxxxxx> wrote:
>>> @@ -279,6 +282,7 @@ LIBOSINFO_0.2.0 {
>>> osinfo_install_scriptlist_new_intersection;
>>> osinfo_install_scriptlist_new_copy;
>>> osinfo_install_scriptlist_get_type;
>>> + osinfo_install_scriptlist_get_by_profile;
>>
>> Unrelated change?
>
> It is used by osinfo-install-script tool.
> Do you think that would be better put it in another commit?
Doesn't matter whats its used by, its not related to this change so
yes, a separate patch for that please.
>>> @@ -210,6 +224,19 @@ osinfo_install_script_class_init (OsinfoInstallScriptClass *klass)
>>> PROP_PROFILE,
>>> pspec);
>>>
>>> + pspec = g_param_spec_string("output-filename",
>>> + "Output Filename",
>>> + "Output filename for the script",
>>
>> You forgot to change these strings.
>
> Change or remove? :)
Yes, remove. Missed the fact that you added the "output-prefix" prop above.
>>> @@ -308,6 +359,24 @@ static void osinfo_install_script_generate_free(struct OsinfoInstallScriptGenera
>>> g_free(data);
>>> }
>>>
>>> +struct OsinfoInstallScriptGenerateOutput {
>>
>> Declare as 'static' and a 'Data' suffix.
>
> Should I do the same for OsinfoInstallScriptGenerate as well?
Yes.
>>
>>> + GSimpleAsyncResult *res;
>>> + GCancellable *cancellable;
>>> + GError *error;
>>> + GFile *file;
>>> + GFileOutputStream *stream;
>>> + gchar *output;
>>> + gssize output_len;
>>> + gssize output_pos;
>>> +};
>>> +
>>> +static void osinfo_install_script_generate_output_free(struct OsinfoInstallScriptGenerateOutput *data)
>>> +{
>>> + g_object_unref(data->stream);
>>> + g_object_unref(data->res);
>>> + g_free(data);
>>> +}
>>> +
>>>
>>> static xsltStylesheetPtr osinfo_install_script_load_template(const gchar *uri,
>>> const gchar *template,
>>> @@ -621,9 +690,9 @@ void osinfo_install_script_generate_async(OsinfoInstallScript *script,
>>> }
>>> }
>>>
>>> -gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>> - GAsyncResult *res,
>>> - GError **error)
>>> +static gpointer osinfo_install_script_generate_finish_common(OsinfoInstallScript *script,
>>> + GAsyncResult *res,
>>> + GError **error)
>>> {
>>> GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(res);
>>>
>>> @@ -635,13 +704,61 @@ gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>> return g_simple_async_result_get_op_res_gpointer(simple);
>>> }
>>>
>>> +gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>> + GAsyncResult *res,
>>> + GError **error)
>>> +{
>>> + return osinfo_install_script_generate_finish_common(script,
>>> + res,
>>> + error);
>>> +}
>>> +
>>> +GFile *osinfo_install_script_generate_output_finish(OsinfoInstallScript *script,
>>> + GAsyncResult *res,
>>> + GError **error)
>>
>> So the function with output suffix gives you a GFile while the one w/o
>> it gives you the path? I'd suggest only having one function named
>> 'osinfo_install_script_generate' that returns the GFile. Its very easy
>> to get name from GFile if app needs that.
>
> Sorry, probably I'm missing something.
> But we are, in the both cases, returning a GFile, no?
Huh? You are returning a string from osinfo_install_script_generate()
and osinfo_install_script_generate_finish().
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
