randy3k / AlignTab

An alignment plugin for Sublime Text using regular expression
MIT License
631 stars 25 forks source link

added support for multiple regex patterns #59

Closed KroniK907 closed 8 years ago

KroniK907 commented 8 years ago

This allows for a user to call user_input as a list with multiple regex patterns which are then called sequentially. user_input can still be called as a string, so no previous functionality is lost. I also set up the [Patterns not Found:] box to show the patterns which were not found since the user may not know which regex pattern failed if they run multiple.

KroniK907 commented 8 years ago

Added the match_all argument that was mentioned in #39

KroniK907 commented 8 years ago

I think that was the final commit for this branch. I updated the functionality of the named_patterns to allow for lists within each named pattern, as well as allowing a user to call user_input containing a list of named patterns.

So for example, let's say we have the following named patterns defined:

[
  "named_patterns": {
    "pattern1": [ "regex1", "regex2" ],
    "pattern2": [ "regex4", "regex5" ]
  }
]

Then if we had a keybind like:

[
  {
    "keys: ["ctrl+r"],
    "command": "align_tab",
    "args": {
      "user_input": [ "pattern1", "regex3", "pattern2", "regex6" ]
    }
  }
]

The result would be:

user_input = [ "regex1", "regex2", "regex3", "regex4", "regex5", "regex6" ]

So essentially exactly as the user expects.

randy3k commented 8 years ago

I think that expanding [ "pattern1", "regex3", "pattern2", "regex6" ] to [ "regex1", "regex2", "regex3", "regex4", "regex5", "regex6" ] does not have any practical usage. It would be much transparent and clear to list out all the inputs.

However, I think named_patterns should work on the multiple regex.

  "named_patterns": {
    "js" :  [ "input1",  "input2"]
    }

it would be even better to even support "match_all" flag.

  "named_patterns": {
    "js" :  [ "input1",  "input2", true]
KroniK907 commented 8 years ago

While I do think mixing manual regex and the named_patterns would be rather silly, I think in general it's nice to allow users to do what they want and allow it to work, even if it's not "optimal." As for passing a flag as one of the items, I guess that might work, however that kinda breaks the "explicit is better than implicit" concept of python.

Overall I think the match_all state should be defined where the user makes the actual call for the align_tab command. The named_patterns should serve as only a way to organize groups of regex. When a user wants to call one of those named patterns they must call that named pattern from somewhere else. The match_all argument should probably then be called wherever the align_tab command is run from. So in this way, you could set up a single named pattern and then have two keybinds. One with the match_all as true and one with match_all as false. This keeps the user from accidently defining the match_all argument as true in the named patterns file and then attempting to call it later with the match_all as false.

Those are just my thoughts on it.

randy3k commented 8 years ago

I understand your thoughts.

However, I think the "named_patterns" functionality should only be used in the interactive input panel. These are no advantages of using them in the keybinds, as you say "explicit is better than implicit".

For the match_all option, it may be of interest to trigger it in the input panel. Putting it as an argument of "align_tab" does not serve this purpose. Unless we complicate the already complicated "regex/options" pattern, for example,

user_input = [ "regex1/l0r0!", "regex2/!", "regex3" ]

The ! flag would stop further matches if the current regex has a match.

KroniK907 commented 8 years ago

I like that idea. I had kinda sorta forgot about the input panel :smile:

I still think calling named_patterns from a key bind or context menu should be allowed but the rest of what you stated above I completely agree with.

randy3k commented 8 years ago

One of the reasons of not allowing named_patterns in keybind is to avoid infinite loop as follows

  "named_patterns": {
    "js" :  [ "input1",  "js"]
    }

Of course, you could control of the number of recursions and only restrict the replacement at the top-level.

randy3k commented 8 years ago

@KroniK907 I have merged your first commit 45d5af54659f0686b30c0993ae1ddb17958e72a5. The match_all option related commits would need some more thoughts before merging them.