Little bit of a tidy up. Code should be more efficient this way, and less

off it to make my eyes bleed.  Also removed the risk of ever getting an
infinite loop on weird occasions.

src/move_messages.php

@@ -30,20 +30,20 @@ if ( !sqgetGlobalVar('composesession', $composesession, SQ_SESSION) ) {
 function attachSelectedMessages($msg, $imapConnection) {
     global $username, $attachment_dir, $startMessage,
            $data_dir, $composesession, $uid_support,
-	   $msgs, $thread_sort_messages, $allow_server_sort, $show_num,
-	   $compose_messages;
+       $msgs, $thread_sort_messages, $allow_server_sort, $show_num,
+       $compose_messages;
 
     if (!isset($compose_messages)) {
-	    $compose_messages = array();
+        $compose_messages = array();
             sqsession_register($compose_messages,'compose_messages');
     }
 
     if (!$composesession) {
-	    $composesession = 1;
-            sqsession_register($composesession,'composesession');  	    
+        $composesession = 1;
+            sqsession_register($composesession,'composesession');
     } else {
-	    $composesession++;
-	    sqsession_register($composesession,'composesession');  	    
+        $composesession++;
+        sqsession_register($composesession,'composesession');
     }
 
     $hashed_attachment_dir = getHashedDir($username, $attachment_dir, $composesession);
@@ -65,28 +65,28 @@ function attachSelectedMessages($msg, $imapConnection) {
 
     while ($j < count($msg)) {
         if (isset($msg[$i])) {
-    	    $id = $msg[$i];
-    	    $body_a = sqimap_run_command($imapConnection, "FETCH $id RFC822",true, $response, $readmessage, $uid_support);
-    	    if ($response == 'OK') {
-	        $k = $i + $start_index;
-		$subject = $msgs[$k]['SUBJECT'];
-    
-        	array_shift($body_a);
-        	$body = implode('', $body_a);
-        	$body .= "\r\n";
-        		
-        	$localfilename = GenerateRandomString(32, 'FILE', 7);
-        	$full_localfilename = "$hashed_attachment_dir/$localfilename";
-        	
-        	$fp = fopen( $full_localfilename, 'wb');
-        	fwrite ($fp, $body);
-        	fclose($fp);
-	        $composeMessage->initAttachment('message/rfc822',$subject.'.eml', 
-	                 $full_localfilename);
-    	    }
-	    $j++;
-	}
-	$i++;	
+            $id = $msg[$i];
+            $body_a = sqimap_run_command($imapConnection, "FETCH $id RFC822",true, $response, $readmessage, $uid_support);
+            if ($response == 'OK') {
+            $k = $i + $start_index;
+        $subject = $msgs[$k]['SUBJECT'];
+
+            array_shift($body_a);
+            $body = implode('', $body_a);
+            $body .= "\r\n";
+
+            $localfilename = GenerateRandomString(32, 'FILE', 7);
+            $full_localfilename = "$hashed_attachment_dir/$localfilename";
+
+            $fp = fopen( $full_localfilename, 'wb');
+            fwrite ($fp, $body);
+            fclose($fp);
+            $composeMessage->initAttachment('message/rfc822',$subject.'.eml',
+                     $full_localfilename);
+            }
+        $j++;
+    }
+    $i++;
     }
     $compose_messages[$composesession] = $composeMessage;
     sqsession_register($compose_messages,'compose_messages');
@@ -142,116 +142,92 @@ $exception = false;
 
 do_hook('move_before_move');
 
+
+/*
+    Move msg list sorting up here, as it is used several times,
+    makes it more efficient to do it in one place for the code
+*/
+$id = array();
+if (isset($msg) && is_array($msg)) {
+    foreach( $msg as $key=>$uid ) {
+        // using foreach removes the risk of infinite loops that was there //
+        $id[] = $uid;
+    }
+}
+
 // expunge-on-demand if user isn't using move_to_trash or auto_expunge
 if(isset($expungeButton)) {
     $cnt = sqimap_mailbox_expunge($imapConnection, $mailbox, true);
     if (($startMessage+$cnt-1) >= $mbx_response['EXISTS']) {
         if ($startMessage > $show_num) {
-	    $location = set_url_var($location,'startMessage',$startMessage-$show_num,false);
-	} else {
-	    $location = set_url_var($location,'startMessage',1,false);
-	}
+            $location = set_url_var($location,'startMessage',$startMessage-$show_num,false);
+        } else {
+            $location = set_url_var($location,'startMessage',1,false);
+        }
     }
 } elseif(isset($undeleteButton)) {
     // undelete messages if user isn't using move_to_trash or auto_expunge
-    if (is_array($msg) == 1) {
-        // Removes \Deleted flag from selected messages
-        $j = $i = 0;
-        $id = array();
-        // If they have selected nothing msg is size one still, but will be an infinite
-        //    loop because we never increment j.  so check to see if msg[0] is set or not to fix this.
-        while ($j < count($msg)) {
-            if ($msg[$i]) {
-	        $id[] = $msg[$i];
-                $j++;
-            }
-            $i++;
-        }
-	if (count($id)) {
-            sqimap_toggle_flag($imapConnection, $id, '\\Deleted',false,true);
-        }
+    // Removes \Deleted flag from selected messages
+    if (count($id)) {
+        sqimap_toggle_flag($imapConnection, $id, '\\Deleted',false,true);
     } else {
-	$exception = true;
+        $exception = true;
     }
 } elseif (!isset($moveButton)) {
-    // If the delete button was pressed, the moveButton variable will not be set.
-    if (is_array($msg)) {
-        // Marks the selected messages as 'Deleted'
-        $j = $i = $cnt = 0;
-	$id = array();
-        // If they have selected nothing msg is size one still, but will be an infinite
-        //    loop because we never increment j.  so check to see if msg[0] is set or not to fix this.
-        while ($j < count($msg)) {
-            if (isset($msg[$i])) {
-	        $id[] = $msg[$i];
-                $j++;
+    if (count($id)) {
+        $cnt = count($id);
+        if (!isset($attache)) {
+            if (isset($markRead)) {
+                sqimap_toggle_flag($imapConnection, $id, '\\Seen',true,true);
+            } else if (isset($markUnread)) {
+                sqimap_toggle_flag($imapConnection, $id, '\\Seen',false,true);
+            } else  {
+                sqimap_msgs_list_delete($imapConnection, $mailbox, $id);
+                if ($auto_expunge) {
+                    $cnt = sqimap_mailbox_expunge($imapConnection, $mailbox, true);
+                }
             }
-            $i++;
-        }
-	if (count($id) && !isset($attache)) {
-           if (isset($markRead)) {
-	      sqimap_toggle_flag($imapConnection, $id, '\\Seen',true,true);
-           } else if (isset($markUnread)) {
-	      sqimap_toggle_flag($imapConnection, $id, '\\Seen',false,true);
-           } else  {
-	      sqimap_msgs_list_delete($imapConnection, $mailbox, $id);
-              if ($auto_expunge) {
-                 $cnt = sqimap_mailbox_expunge($imapConnection, $mailbox, true);
-	      }
-           }
         }
         if (isset($attache)) {
-	    $composesession = attachSelectedMessages($msg, $imapConnection);
-	    $location = set_url_var($location, 'session', $composesession, false);
-	    if ($compose_new_win) {
-	        $location = set_url_var($location, 'composenew', 1, false);
-	    } else {
-		$location = str_replace('search.php','compose.php',$location);
-		$location = str_replace('right_main.php','compose.php',$location);
-	    }
-	} else {		
-	    if (($startMessage+$cnt-1) >= $mbx_response['EXISTS']) {
-    	       if ($startMessage > $show_num) {
-	           $location = set_url_var($location,'startMessage',$startMessage-$show_num, false);
-	       } else {
-	  	   $location = set_url_var($location,'startMessage',1, false);
-	       }
-	    }
-        } 
+            $composesession = attachSelectedMessages($id, $imapConnection);
+            $location = set_url_var($location, 'session', $composesession, false);
+            if ($compose_new_win) {
+                $location = set_url_var($location, 'composenew', 1, false);
+            } else {
+                $location = str_replace('search.php','compose.php',$location);
+                $location = str_replace('right_main.php','compose.php',$location);
+            }
+        } else {
+            if (($startMessage+$cnt-1) >= $mbx_response['EXISTS']) {
+                if ($startMessage > $show_num) {
+                    $location = set_url_var($location,'startMessage',$startMessage-$show_num, false);
+                } else {
+                    $location = set_url_var($location,'startMessage',1, false);
+                }
+            }
+        }
     } else {
-    	$exception = true;
+        $exception = true;
     }
 } else {    // Move messages
-    // lets check to see if they selected any messages
-    if (is_array($msg)) {
-        $j = $i = 0;
-	$id = array();
-        // If they have selected nothing msg is size one still, but will be an infinite
-        //    loop because we never increment j.  so check to see if msg[0] is set or not to fix this.
-	$cnt = count($msg);
-        while ($j < $cnt) {
-            if (isset($msg[$i])) {
-	        $id[] = $msg[$i];
-                $j++;
-            }
-            $i++;
-        }
-	sqimap_msgs_list_copy($imapConnection,$id,$targetMailbox);
+
+    if (count($id)) {
+        sqimap_msgs_list_copy($imapConnection,$id,$targetMailbox);
         if ($auto_expunge) {
             $cnt = sqimap_mailbox_expunge($imapConnection, $mailbox, true);
-	} else {
-	    $cnt = 0;
-	}
-	
-	if (($startMessage+$cnt-1) >= $mbx_response['EXISTS']) {
-    	    if ($startMessage > $show_num) {
-		$location = set_url_var($location,'startMessage',$startMessage-$show_num, false);
-	    } else {
-		$location = set_url_var($location,'startMessage',1, false);
-	    }
-	}
+        } else {
+            $cnt = 0;
+        }
+
+        if (($startMessage+$cnt-1) >= $mbx_response['EXISTS']) {
+            if ($startMessage > $show_num) {
+                $location = set_url_var($location,'startMessage',$startMessage-$show_num, false);
+            } else {
+                $location = set_url_var($location,'startMessage',1, false);
+            }
+        }
     } else {
-	$exception = true;
+        $exception = true;
     }
 }
 // Log out this session