Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jpegmeta fixes #381

Merged
merged 2 commits into from Oct 20, 2013
Merged

Jpegmeta fixes #381

merged 2 commits into from Oct 20, 2013

Conversation

Klap-in
Copy link
Collaborator

@Klap-in Klap-in commented Oct 16, 2013

Todo:

  • about line 1511: IDEA intelliJ marks as error: cannot use [] for reading at the argument $meta[]
        if ($values[$i]['tag'] == 'rdf:Bag' || $values[$i]['tag'] == 'rdf:Seq') {
            // Array property
            $meta = array();
            while ($values[++$i]['tag'] == 'rdf:li') {
                $this->_parseXmpNode($values, $i, $meta[], $count);
            }
            $i++; // skip closing Bag/Seq tag

        } elseif ($values[$i]['tag'] == 'rdf:Alt') {

Someone suggestions?

Further there are (i guess) still many possible Only variables should be assigned by reference

I have no data available to test these stuff, so i cannot finish this properly.

@glensc
Copy link
Contributor

glensc commented Oct 16, 2013

what $meta[] is supposed to be doing at all, i mean as an argument, not as assignment

@Chris--S
Copy link
Collaborator

Think of it as shorthand for, creating a new element and having the function update that new element (see code fragment below). Functions with side-effects on their parameters aren't the clearest, can the function be re-written removing the need for passing parameters by reference?

$new_element = null;
$meta[] = &$new;
some_fn_with_side_fx($new);

...

function some_fn_with_side_fx(&$param) {
...
}

@Klap-in
Copy link
Collaborator Author

Klap-in commented Oct 16, 2013

@HakanS can you have a look into the _parseXmpNode function and fix $meta[]? or do you have some data that let me test this a bit complete?

@@ -874,7 +874,7 @@ function save($fileName = "") {
/*************************************************************/

/*************************************************************/
function _dispose() {
function _dispose($fileName = "") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, how did this even work before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is never called.

@splitbrain
Copy link
Collaborator

Maybe it could be rewritten as:

$cnt = count($meta);
while ($values[++$i]['tag'] == 'rdf:li') {
    $meta[$cnt] = null;
    $this->_parseXmpNode($values, $i, $meta[$cnt], $count);
    $cnt++;
}

This library is kind of shitty. I wish we could replace it with something cleaner. Suggestions welcome.

splitbrain added a commit that referenced this pull request Oct 20, 2013
@splitbrain splitbrain merged commit 8c40496 into master Oct 20, 2013
@splitbrain splitbrain deleted the jpegmetafixes branch October 20, 2013 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants