ThFriedrich / lammps_vscode

VSCODE extension for language support of LAMMPS scripts
https://thfriedrich.github.io/lammps_vscode/
GNU General Public License v2.0
44 stars 14 forks source link

Regex improvement for multi-keyword commands #2 #3

Closed arn-all closed 4 years ago

arn-all commented 4 years ago

The hover now supports "fix foo bar box/relax" pattern

arn-all commented 4 years ago

More specifically :

ThFriedrich commented 4 years ago

That's a great improvement. :) However, few things I noticed:

  1. The regex doesn't capture single-word commands with slashes, like "temper/grem"
  2. The splitting should be with a regex too, like: const words = document.getText(range).split(RegExp('[\\t\\s]+'))
  3. Capturing the last word with words[words.length - 1] isn't very robust I think, because you can have trailung keywords and such. In a case like fix 1 all box/relax iso 0.0 vmax 0.001, wouldn't that actually capture "fix 0.0001"? Your previous version was actually better in that regard, but maybe it needs more elaborate(e.g. iterative) selection rules. Maybe something based on this:
    if (!docs?.command) {
        const sub_com = snippet.split(RegExp('[\\t\\s]+')); 
        docs = get_documentation(sub_com[0])
    }

    Having that kind of check running over the possible combinations may be an idea, e.g: first check if get_documentation(sub_com[0] + ' ' + sub_com[3]) returns something, if it doesn't, check get_documentation(sub_com[0] + ' ' + sub_com[2]), and so on... I admit it's probably not very efficient but it may not really be a problem. I am not sure. Most of the time the first check would actually return something I guess. The other cases would take care of expressions with missing parameters. Capturing that may also provide a possibility to show error-squiggles later on. :thinking:

EDIT: Just seen you already fixed the words[words.length - 1] :+1:

arn-all commented 4 years ago

Yes, sorry for the typo !

  1. can be fixed very easily, I was not considering this kind of commands.
  2. Absolutely !

Edit :

  1. Is it all good now for valid lammps commands ? Testing 0 + 2 and 0 + 1 is a great idea. I agree that it should not make it too slow.