<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [PATCH] Feature : extract media in html format"
href="https://bugs.freedesktop.org/show_bug.cgi?id=56843#c7">Comment # 7</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [PATCH] Feature : extract media in html format"
href="https://bugs.freedesktop.org/show_bug.cgi?id=56843">bug 56843</a>
from <span class="vcard"><a class="email" href="mailto:tsdgeos@terra.es" title="Albert Astals Cid <tsdgeos@terra.es>"> <span class="fn">Albert Astals Cid</span></a>
</span></b>
<pre>First of all thanks for the contribution, don't let the comments make you think
your contribution is unwanted, it's just that we try to get the better code
possible for all the features.
The patches still seem a big mixed
<a href="https://bugs.freedesktop.org/attachment.cgi?id=70245">https://bugs.freedesktop.org/attachment.cgi?id=70245</a> and
<a href="https://bugs.freedesktop.org/attachment.cgi?id=70246">https://bugs.freedesktop.org/attachment.cgi?id=70246</a> both seem to have the same
first "patch" inside.
Anyway, could you explain in text what the patch does? I can read the code but
i'd like something longer than the first comment, something like, it saves this
there and bla bla.
Also it would be cool if you could send the patches with the least number of
whitespace changes, it takes a while to have a look at
- HtmlImage(GooString *_fName, GfxState *state)
+ HtmlImage(GooString *_fName, GfxState *state)
or
- Object obj1;
- // valid 'F' key -> external file
- kind = soundExternal;
- if (getFileSpecNameForPlatform (&tmp, &obj1)) {
- fileName = obj1.getString()->copy();
- obj1.free();
- }
+ Object obj1;
+ // valid 'F' key -> external file
+ kind = soundExternal;
+ if (getFileSpecNameForPlatform (&tmp, &obj1)) {
+ fileName = obj1.getString()->copy();
+ obj1.free();
+ }
and realize the only thing that has changed is the leading space, so it helps
making the review faster if these kind of change is not present
Also can you explain the code change in poppler/FileSpec.cc?
In AnnotRichMediaInstance:
* you have the name variable that is never used
* type and filespec are not set to NULL if the objects are not present
* You want to strdup or create a goostring from type since once the Object
runs out of references you'll have a pointer pointing to free'd memory
* Please make subtype be an enum and not an int
In AnnotRichMediaConfiguration:
* You never free the instances list
* Same thing for the subtype enum thing
* Same thing for the strduping of char* you get from a getName
Please fix these mistakes and post the new patches and i'll continue with the
review.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>