search

vim / vim

The official Vim repository
https://www.vim.org
Vim License
36.13k stars 5.41k forks source link

vim9script: confusing error when using = on undefined variable #10899

Open arp242 opened 2 years ago

arp242 commented 2 years ago 
% cat err
vim9script
tabs_len = 2

% vim -Nu NONE +source\ err
Error detected while processing command line..script /home/martin/err:
line    2:
E1144: Command "tabs" is not followed by white space: tabs_len = 2
Press ENTER or type command to continue

I copy/pasted some code from somewhere else, and the var tabs_len got lost. Also happens with +=, -=, etc.

If I change the variable name to xxx it's a little bit better:

    Error detected while processing command line..script /home/martin/err:
    line    2:
    E492: Not an editor command: xxx = 2
    Press ENTER or type command to continue

But ideally, "undefined variable: tabs_len" or the like would be best, but I'm guessing there may be a reason it doesn't do that already. Either way, the white space error is very confusing.

Vim 9.0.0192

lacygoill commented 2 years ago

But ideally, "undefined variable: tabs_len" or the like would be best, but I'm guessing there may be a reason it doesn't do that already.

The difference between tabs_len and xxx, is that the former contains a prefix which matches a known Ex command, while the latter does not (:x is invalid in Vim9).

As a result, there is an ambiguity in the semantics of tabs_len. Is it a variable, or is it the :tabs command to which you pass the _len argument? Here, since Vim has never seen a declaration for a variable tabs_len, it infers that you're trying to execute :tabs, because that's the only remaining semantics.

However, I agree that the error is a bit confusing, because it suggests to add a space which wouldn't help here, since :tabs doesn't accept any argument.

In this specific case, we might give an E121: Undefined variable error. But not in the general case where the variable might contain a prefix matching a known Ex command which does accept an argument. For example, as documented at :help E1144:

vim9script
exit_cb = 123
E1144: Command "exit" is not followed by white space: exit_cb = 123

Here, we can't give E121: Undefined variable, because it might be that you wanted to execute :exit _cb (notice the space).

Either way, the white space error is very confusing.

This started with patch 8.2.2138. Before that, it was even worse, because Vim didn't enforce a space between a command and its argument, which could lead to no error being given and an unexpected command being run. Now, we no longer have any unexpected command being run, and we do have an error. But we might need to make it easier to understand when possible (like here).

brammool commented 2 years ago

Steps to reproduce

% cat err vim9script tabs_len = 2

% vim -Nu NONE +source\ err Error detected while processing command line..script /home/martin/err: line 2: E1144: Command "tabs" is not followed by white space: tabs_len = 2 Press ENTER or type command to continue

I copy/pasted some code from somewhere else, and the var tabs_len got lost. Also happens with +=, -=, etc.

If I change the variable name to xxx it's a little bit better:

    Error detected while processing command line..script /home/martin/err:
    line    2:
    E492: Not an editor command: xxx = 2
    Press ENTER or type command to continue

But ideally, "undefined variable: tabs_len" or the like would be best, but I'm guessing there may be a reason it doesn't do that already. Either way, the white space error is very confusing.

As lacygoill mentioned: we do not know that you were using a variable name. Vim just see ":tabs" and something following.

We could make an exception for using "_", to give a different error message. But it's just one case. What if you used "tabs2 = 2"?

Another guess is that "=" follows. But it could also be "+=" or "->" or "(args)" (used for a function reference with a typo).

I think the error is clear enough, since it mentions it recognized "tabs" as a command. Just takes a bit of background knowledge to understand it.

-- E M A C S s e l o h c t t n i a a t f p r t e o l

/// Bram Moolenaar -- @.*** -- http://www.Moolenaar.net \\ /// \\ \\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\ help me help AIDS victims -- http://ICCF-Holland.org ///

arp242 commented 2 years ago

Does Vim really need to check if it's the :tabs command if spaces after it are mandatory? It's allowed in legacy VimScript and I've seen it on a few (rare) occasions, but even when migrating I'd consider tabs_len not defined to be clearer, but also cases where you do want to pass an argument, like say:

vim9script
tabmove1

"undefined reference: tabmove1" seems clearer to me. People can figure out that they need to add a space, because that's how pretty much everything works. Although there are still other cases where this kind of "forgot to use var" error goes wrong, for example:

first = 1

Will try to run :first.

Not sure that can easily be improved on. But that's a different issue.

Just takes a bit of background knowledge to understand it.

Personally I'd say the thing with VimScript is that very few people have this knowledge. Very few people write VimScript for a living outside of the very occasional plugin to interface with $product, like GitHub Copilot, and there's a comparatively small community of plugin authors and Vim aficionados. Most "normal" people I've use VimScript once in a blue moon, at most.

lacygoill commented 2 years ago

How about we append (missing var?) at the end of the message?

Before:

E1144: Command "tabs" is not followed by white space: tabs_len = 2

After:

E1144: Command "tabs" is not followed by white space: tabs_len = 2 (missing var?)
                                                                   ^------------^
diff --git a/src/errors.h b/src/errors.h
index 8aad956d9..cb7eb9df6 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -2925,7 +2925,7 @@ EXTERN char e_calling_test_garbagecollect_now_while_v_testing_is_not_set[]
 EXTERN char e_empty_expression_str[]
    INIT(= N_("E1143: Empty expression: \"%s\""));
 EXTERN char e_command_str_not_followed_by_white_space_str[]
-   INIT(= N_("E1144: Command \"%s\" is not followed by white space: %s"));
+   INIT(= N_("E1144: Command \"%s\" is not followed by white space: %s (missing var?)"));
 EXTERN char e_missing_heredoc_end_marker_str[]
    INIT(= N_("E1145: Missing heredoc end marker: %s"));
 EXTERN char e_command_not_recognized_str[]

Would it help? Would it make sense in the general case?

lacygoill commented 2 years ago

That's what we do in E1100: https://github.com/vim/vim/blob/326c5d36e7cb8526330565109c17b4a13ff790ae/src/errors.h#L2836

The previous patch would make E1144 inconsistent with E1100. New patch to make it consistent:

diff --git a/src/errors.h b/src/errors.h
index 8aad956d9..78ccda660 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -2925,7 +2925,7 @@ EXTERN char e_calling_test_garbagecollect_now_while_v_testing_is_not_set[]
 EXTERN char e_empty_expression_str[]
    INIT(= N_("E1143: Empty expression: \"%s\""));
 EXTERN char e_command_str_not_followed_by_white_space_str[]
-   INIT(= N_("E1144: Command \"%s\" is not followed by white space: %s"));
+   INIT(= N_("E1144: Command \"%s\" is not followed by white space (missing :var?): %s"));
 EXTERN char e_missing_heredoc_end_marker_str[]
    INIT(= N_("E1145: Missing heredoc end marker: %s"));
 EXTERN char e_command_not_recognized_str[]
lacygoill commented 2 years ago

Maybe we should add the or conjunction to make it clear that it's another possible cause for the error (and not further info about the previous cause):

diff --git a/src/errors.h b/src/errors.h
index 8aad956d9..d1e764260 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -2833,7 +2833,7 @@ EXTERN char e_string_list_or_blob_required[]
 EXTERN char e_unknown_error_while_executing_str[]
    INIT(= N_("E1099: Unknown error while executing %s"));
 EXTERN char e_command_not_supported_in_vim9_script_missing_var_str[]
-   INIT(= N_("E1100: Command not supported in Vim9 script (missing :var?): %s"));
+   INIT(= N_("E1100: Command not supported in Vim9 script (or missing :var?): %s"));
 EXTERN char e_cannot_declare_script_variable_in_function_str[]
    INIT(= N_("E1101: Cannot declare a script variable in a function: %s"));
 EXTERN char e_lambda_function_not_found_str[]
@@ -2925,7 +2925,7 @@ EXTERN char e_calling_test_garbagecollect_now_while_v_testing_is_not_set[]
 EXTERN char e_empty_expression_str[]
    INIT(= N_("E1143: Empty expression: \"%s\""));
 EXTERN char e_command_str_not_followed_by_white_space_str[]
-   INIT(= N_("E1144: Command \"%s\" is not followed by white space: %s"));
+   INIT(= N_("E1144: Command \"%s\" is not followed by white space (or missing :var?): %s"));
 EXTERN char e_missing_heredoc_end_marker_str[]
    INIT(= N_("E1145: Missing heredoc end marker: %s"));
 EXTERN char e_command_not_recognized_str[]
brammool commented 2 years ago

Maybe we should add the or conjunction to make it clear that it's another possible cause for the error (and not further info about the previous error):

diff --git a/src/errors.h b/src/errors.h
index 8aad956d9..d1e764260 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -2833,7 +2833,7 @@ EXTERN char e_string_list_or_blob_required[]
 EXTERN char e_unknown_error_while_executing_str[]
  INIT(= N_("E1099: Unknown error while executing %s"));
 EXTERN char e_command_not_supported_in_vim9_script_missing_var_str[]
- INIT(= N_("E1100: Command not supported in Vim9 script (missing :var?): %s"));
+ INIT(= N_("E1100: Command not supported in Vim9 script (or missing :var?): %s"));
 EXTERN char e_cannot_declare_script_variable_in_function_str[]
  INIT(= N_("E1101: Cannot declare a script variable in a function: %s"));
 EXTERN char e_lambda_function_not_found_str[]
@@ -2925,7 +2925,7 @@ EXTERN char e_calling_test_garbagecollect_now_while_v_testing_is_not_set[]
 EXTERN char e_empty_expression_str[]
  INIT(= N_("E1143: Empty expression: \"%s\""));
 EXTERN char e_command_str_not_followed_by_white_space_str[]
- INIT(= N_("E1144: Command \"%s\" is not followed by white space: %s"));
+ INIT(= N_("E1144: Command \"%s\" is not followed by white space (or missing :var?): %s"));
 EXTERN char e_missing_heredoc_end_marker_str[]
  INIT(= N_("E1145: Missing heredoc end marker: %s"));
 EXTERN char e_command_not_recognized_str[]

The "missing var?" should only be given if the non-white character can be used in a variable. "tabs_name" could be a variable, but "tabs/name" looks more like a command with an invalid argument. Mentioning "var" would then only add to the confusion.

-- I'd like to meet the man who invented sex and see what he's working on now.

/// Bram Moolenaar -- @.*** -- http://www.Moolenaar.net \\ /// \\ \\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\ help me help AIDS victims -- http://ICCF-Holland.org ///