From 1a59346b51e9ac5a89e303374f4a7ce1647d4c98 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 3 Sep 2012 13:24:01 -0700 Subject: [PATCH] Changed "elseif" to "else if" --- builtin.cpp | 1 - builtin.h | 4 ++ doc_src/if.txt | 4 +- fish_tests.cpp | 8 ++- parser.cpp | 133 +++++++++++++++++++++++++++++++------------- parser.h | 2 +- parser_keywords.cpp | 2 - parser_keywords.h | 2 +- tests/test7.in | 19 +++---- 9 files changed, 117 insertions(+), 58 deletions(-) diff --git a/builtin.cpp b/builtin.cpp index 8c3471114..c1f0cc2e9 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -3792,7 +3792,6 @@ static const builtin_data_t builtin_datas[]= { L"count", &builtin_count, N_( L"Count the number of arguments" ) }, { L"echo", &builtin_echo, N_( L"Print arguments" ) }, { L"else", &builtin_else, N_( L"Evaluate block if condition is false" ) }, - { L"elseif", &builtin_generic, N_( L"Evaluate block if this condition is true but all previous were false" ) }, { L"emit", &builtin_emit, N_( L"Emit an event" ) }, { L"end", &builtin_end, N_( L"End a block of commands" ) }, { L"exec", &builtin_generic, N_( L"Run command in current process" ) }, diff --git a/builtin.h b/builtin.h index eee03b90c..e4683f001 100644 --- a/builtin.h +++ b/builtin.h @@ -73,6 +73,10 @@ enum #define BUILTIN_FOR_ERR_NAME _( L"%ls: '%ls' is not a valid variable name\n" ) +/** Error messages for 'else if' */ +#define BUILTIN_ELSEIF_ERR_COUNT _( L"%ls: can only take 'if' and then another command as an argument\n") +#define BUILTIN_ELSEIF_ERR_ARGUMENT _( L"%ls: any second argument must be 'if'\n") + /** Error message when too many arguments are supplied to a builtin */ diff --git a/doc_src/if.txt b/doc_src/if.txt index aa43bdfb1..dae82ca6c 100644 --- a/doc_src/if.txt +++ b/doc_src/if.txt @@ -1,7 +1,7 @@ \section if if - conditionally execute a command \subsection if-synopsis Synopsis -if CONDITION; COMMANDS_TRUE...; [elseif CONDITION2; COMMANDS_TRUE2...;] [else; COMMANDS_FALSE...;] end +if CONDITION; COMMANDS_TRUE...; [else if CONDITION2; COMMANDS_TRUE2...;] [else; COMMANDS_FALSE...;] end \subsection if-description Description @@ -24,7 +24,7 @@ variable. 
 if test -f foo.txt
 	echo foo.txt exists
-elseif test -f bar.txt
+else if test -f bar.txt
 	echo bar.txt exists
 else
 	echo foo.txt and bar.txt do not exist
diff --git a/fish_tests.cpp b/fish_tests.cpp
index 0d0c5f9b4..438254b4b 100644
--- a/fish_tests.cpp
+++ b/fish_tests.cpp
@@ -434,9 +434,13 @@ static void test_parser()
 	{
 		err( L"'else' command outside of conditional block context undetected" );		
 	}
-	if( !parser.test( L"elseif", 0, 0, 0 ) )
+	if( !parser.test( L"else if", 0, 0, 0 ) )
 	{
-		err( L"'elseif' command outside of conditional block context undetected" );		
+		err( L"'else if' command outside of conditional block context undetected" );		
+	}
+	if( !parser.test( L"if false; else if; end", 0, 0, 0 ) )
+	{
+		err( L"'else if' missing command undetected" );
 	}
 
 	if( !parser.test( L"break", 0, 0, 0 ) )
diff --git a/parser.cpp b/parser.cpp
index 6c1951995..ab79bc4ba 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -146,7 +146,12 @@ The fish parser. Contains functions for parsing and evaluating code.
 /**
    Error when using else builtin outside of if block
 */
-#define INVALID_ELSE_OR_ELSEIF_ERR_MSG _( L"'%ls' builtin not inside of if block" )
+#define INVALID_ELSE_ERR_MSG _( L"'%ls' builtin not inside of if block" )
+
+/**
+   Error when using 'else if' past a naked 'else'
+*/
+#define INVALID_ELSEIF_PAST_ELSE_ERR_MSG _( L"'%ls' used past terminating 'else'" )
 
 /**
    Error when using end builtin outside of block
@@ -1408,11 +1413,6 @@ void parser_t::parse_job_argument_list( process_t *p,
 					{
 						skip=0;
 					}
-                    else if (type == IF && args.at(0).completion == L"elseif")
-                    {
-                        //skip = 0;
-                        printf("SKIP elseif???\n");
-                    }
 				}
 
 				if( !skip )
@@ -1898,19 +1898,37 @@ int parser_t::parse_job( process_t *p,
 			is_new_block=1;
 			consumed = true;
 		}
-        else if( nxt == L"elseif" )
+        else if (nxt == L"else")
         {
-            /* Piggyback on the if block, but we need the elseif command */
-            job_set_flag( j, JOB_ELSEIF, 1 );
-            tok_next( tok );
-            consumed = true;
-            
-            /* Determine if we need to unskip */
-            if (current_block->type() == IF)
+            /* Record where the else is for error reporting */
+            const int else_pos = tok_get_pos(tok);
+            /* See if we have any more arguments, that is, whether we're ELSE IF ... or just ELSE. */
+            tok_next(tok);
+            if (tok_last_type(tok) == TOK_STRING && current_block->type() == IF)
             {
                 const if_block_t *ib = static_cast(current_block);
-                /* We want to execute this ELSEIF if the IF expression was evaluated, it failed, and so has every other ELSEIF (if any) */
-                unskip = (ib->if_expr_evaluated && ! ib->any_branch_taken);
+                
+                /* If we've already encountered an else, complain */
+                if (ib->else_evaluated)
+                {
+                    error( SYNTAX_ERROR,
+                           else_pos,
+                           INVALID_ELSEIF_PAST_ELSE_ERR_MSG,
+                           L"else if");
+
+                }
+                else
+                {
+                    
+                    job_set_flag( j, JOB_ELSEIF, 1 );
+                    consumed = true;
+                    
+                    /* We're at the IF. Go past it. */
+                    tok_next(tok);
+                    
+                    /* We want to execute this ELSEIF if the IF expression was evaluated, it failed, and so has every other ELSEIF (if any) */
+                    unskip = (ib->if_expr_evaluated && ! ib->any_branch_taken);
+                }
             }
         }
 
@@ -2489,7 +2507,9 @@ void parser_t::eval_job( tokenizer *tok )
                         /* Execute the IF */
                         bool if_result = (proc_get_last_status() == 0);
                         ib->any_branch_taken = if_result;
-						current_block->skip = ! if_result; //don't execute if the expression failed
+                        
+                        /* Don't execute if the expression failed */
+						current_block->skip = ! if_result;
 						ib->if_expr_evaluated = true;
                     }
                     else if (ib->is_elseif_entry && ! ib->any_branch_taken)
@@ -2962,7 +2982,7 @@ int parser_t::test( const  wchar_t * buff,
 	/* 
 	   Set to one if an additional process specification is needed 
 	*/
-	int needs_cmd=0; 
+	bool needs_cmd = false;
 
 	/*
 	  Counter on the number of arguments this function has encountered
@@ -3050,7 +3070,7 @@ int parser_t::test( const  wchar_t * buff,
 							}
 						}
 						
-						needs_cmd=0;
+						needs_cmd = false;
 					}
 					
 					/*
@@ -3066,9 +3086,9 @@ int parser_t::test( const  wchar_t * buff,
 					/*
 					  Store the block level. This needs to be done
 					  _after_ checking for end commands, but _before_
-					  shecking for block opening commands.
+					  checking for block opening commands.
 					*/
-					bool is_else_or_elseif = (command == L"else" || command == L"elseif");
+					bool is_else_or_elseif = (command == L"else");
 					if( block_level )
 					{
 						block_level[tok_get_pos( &tok )] = count + (is_else_or_elseif?-1:0);
@@ -3107,7 +3127,7 @@ int parser_t::test( const  wchar_t * buff,
 					*/
 					if( parser_keywords_is_subcommand( command ) && !parser_keywords_skip_arguments(command ) )
 					{
-						needs_cmd = 1;
+						needs_cmd = true;
 						had_cmd = 0;
 					}
 					
@@ -3231,8 +3251,6 @@ int parser_t::test( const  wchar_t * buff,
 								}
 							}
 						}
-
-
 					}
 					
 
@@ -3289,13 +3307,12 @@ int parser_t::test( const  wchar_t * buff,
 								}
 							}
 						}
-						
 					}
 
 					/*
-					  Test that else and elseif are only used directly in an if-block
+					  Test that else and else-if are only used directly in an if-block
 					*/
-					if( command == L"else" || command == L"elseif" )
+					if( command == L"else")
 					{
 						if( !count || block_type[count-1]!=IF )
 						{
@@ -3304,16 +3321,12 @@ int parser_t::test( const  wchar_t * buff,
 							{
 								error( SYNTAX_ERROR,
 									   tok_get_pos( &tok ),
-									   INVALID_ELSE_OR_ELSEIF_ERR_MSG,
+									   INVALID_ELSE_ERR_MSG,
                                        command.c_str());
 
 								print_errors( *out, prefix );
-                                
-                                /* Don't complain about elseif missing a command for elseif if we already complained about elseif being out of place */
-                                if (needs_cmd) had_cmd = true;
 							}
 						}
-
 					}
 
 					/*
@@ -3336,12 +3349,10 @@ int parser_t::test( const  wchar_t * buff,
 					
 				}
 				else
-				{
+				{                    
 					err |= parser_test_argument( tok_last( &tok ), out, prefix, tok_get_pos( &tok ) );
 					
-					/*
-					  If possible, keep track of number of supplied arguments
-					*/
+					/* If possible, keep track of number of supplied arguments */
 					if( arg_count >= 0 && expand_is_clean( tok_last( &tok ) ) )
 					{
 						arg_count++;
@@ -3398,6 +3409,32 @@ int parser_t::test( const  wchar_t * buff,
 								}
 							}
 						}
+                        else if (command == L"else")
+                        {
+                            if (arg_count == 1)
+                            {
+                                /* Any second argument must be "if" */
+								if (wcscmp(tok_last(&tok), L"if") != 0)
+								{
+									err = 1;
+									
+									if( out )
+									{
+										error( SYNTAX_ERROR,
+											   tok_get_pos( &tok ),
+											   BUILTIN_ELSEIF_ERR_ARGUMENT,
+											   L"else" );
+										print_errors( *out, prefix );
+									}
+								}
+                                else
+                                {
+                                    /* Successfully detected "else if". Now we need a new command. */
+                                    needs_cmd = true;
+                                    had_cmd = false;
+                                }
+                            }
+                        }
 					}
 
 				}
@@ -3439,7 +3476,7 @@ int parser_t::test( const  wchar_t * buff,
 						print_errors( *out, prefix );
 					}
 				}
-				needs_cmd=0;
+				needs_cmd = false;
 				had_cmd = 0;
 				is_pipeline=0;
 				forbid_pipeline=0;
@@ -3488,7 +3525,7 @@ int parser_t::test( const  wchar_t * buff,
 				}
 				else
 				{
-					needs_cmd=1;
+					needs_cmd = true;
 					is_pipeline=1;
 					had_cmd=0;
 					end_of_cmd = 1;
@@ -3576,8 +3613,26 @@ int parser_t::test( const  wchar_t * buff,
 						print_errors( *out, prefix );
 					}
 				}
-				
 			}
+            else if (has_command && command == L"else")
+            {
+                if (arg_count == 1)
+                {
+                    /* If we have any arguments, we must have at least two...either "else" or "else if foo..." */
+                    err = true;
+                    if (out)
+                    {
+						error( SYNTAX_ERROR,
+							   tok_get_pos( &tok ),
+							   BUILTIN_ELSEIF_ERR_COUNT,
+							   L"else",
+							   arg_count );
+						
+						print_errors( *out, prefix );
+
+                    }
+                }
+            }
 			
 		}
 		
diff --git a/parser.h b/parser.h
index de59a0d16..331ca520d 100644
--- a/parser.h
+++ b/parser.h
@@ -141,7 +141,7 @@ struct block_t
 struct if_block_t : public block_t
 {
     bool if_expr_evaluated; // whether we've evaluated the if expression
-    bool is_elseif_entry; // whether we're at the beginning of an active branch (IF or ELSEIF)
+    bool is_elseif_entry; // whether we're at the beginning of an ELSEIF branch
     bool any_branch_taken; // whether the clause of the if statement or any elseif has been found to be true
     bool else_evaluated; // whether we've encountered a terminal else block
     
diff --git a/parser_keywords.cpp b/parser_keywords.cpp
index 8a5f1b742..efc2ff6e6 100644
--- a/parser_keywords.cpp
+++ b/parser_keywords.cpp
@@ -43,7 +43,6 @@ bool parser_keywords_is_subcommand( const wcstring &cmd )
 				  L"while",
 				  L"exec",
 				  L"if",
-                  L"elseif",
 				  L"and",
 				  L"or",
 				  L"not" );
@@ -69,7 +68,6 @@ bool parser_keywords_is_reserved( const wcstring &word)
 				  L"end",
 				  L"case",
 				  L"else",
-				  L"elseif",
 				  L"return",
 				  L"continue",
 				  L"break" );
diff --git a/parser_keywords.h b/parser_keywords.h
index 803df49d5..5bad85aab 100644
--- a/parser_keywords.h
+++ b/parser_keywords.h
@@ -26,7 +26,7 @@ bool parser_keywords_is_switch( const wcstring &cmd );
 
 
 /**
-   Tests if the specified commands parameters should be interpreted as another command, which will be true if the command is either 'command', 'exec', 'if', 'while', 'elseif', or 'builtin'.  
+   Tests if the specified commands parameters should be interpreted as another command, which will be true if the command is either 'command', 'exec', 'if', 'while', or 'builtin'.  This does not handle "else if" which is more complicated.
 
    \param cmd The command name to test
    \return 1 of the command parameter is a command, 0 otherwise
diff --git a/tests/test7.in b/tests/test7.in
index 60cc13859..8e80da59e 100644
--- a/tests/test7.in
+++ b/tests/test7.in
@@ -37,15 +37,14 @@ contains -i -- string a b c string d
 contains -i -- -- a b c; or echo nothing
 contains -i -- -- a b c -- v
 
-# Test if, else, and elseif
-# Test if, else, and elseif
+# Test if, else, and else if
 if true
 	echo alpha1.1
 	echo alpha1.2
-elseif false
+else if false
 	echo beta1.1
 	echo beta1.2
-elseif false
+else if false
 	echo gamma1.1
 	echo gamma1.2
 else
@@ -56,10 +55,10 @@ end
 if false
 	echo alpha2.1
 	echo alpha2.2
-elseif begin ; true ; end
+else if begin ; true ; end
 	echo beta2.1
 	echo beta2.2
-elseif begin ; echo nope2.1; false ; end
+else if begin ; echo nope2.1; false ; end
 	echo gamma2.1
 	echo gamma2.2
 else
@@ -70,10 +69,10 @@ end
 if false
 	echo alpha3.1
 	echo alpha3.2
-elseif begin ; echo yep3.1; false ; end
+else if begin ; echo yep3.1; false ; end
 	echo beta3.1
 	echo beta3.2
-elseif begin ; echo yep3.2; true ; end
+else if begin ; echo yep3.2; true ; end
 	echo gamma3.1
 	echo gamma3.2
 else
@@ -84,10 +83,10 @@ end
 if false
 	echo alpha4.1
 	echo alpha4.2
-elseif begin ; echo yep4.1; false ; end
+else if begin ; echo yep4.1; false ; end
 	echo beta4.1
 	echo beta4.2
-elseif begin ; echo yep4.2; false ; end
+else if begin ; echo yep4.2; false ; end
 	echo gamma4.1
 	echo gamma4.2
 else