Skip to content

Conversation

@pr4th4m
Copy link

@pr4th4m pr4th4m commented Dec 15, 2023

This PR adds support multiple boolean flags for one prefix

["."] = {
    flag = "hidden no-ignore ignore-case",
},

This is a hacky way to add support for multiple boolean flags, if you come up with a better option we can collab or if you feel this isn't the way forward we can close this PR

Copy link
Owner

@fdschmidt93 fdschmidt93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Happy to generally have this change.

Nevertheless, shouldn't flag then better be a string or table of strings? Strikes me as cleaner than whitespace-separated flags.

Plus, we'd have to document this somewhere.

@pr4th4m
Copy link
Author

pr4th4m commented Dec 19, 2023

Nevertheless, shouldn't flag then better be a string or table of strings? Strikes me as cleaner than whitespace-separated flags.

Good point, however, flag will have two datatypes string and table as its been used for boolean flags as well as regular (key/value) flags. I feel this might confuse user as the data type of flag will not be consistent across boolean and regular flags. Ofcourse, we can address this in documentation but at first glance it might be confusing, your thoughts ?

ex -

  ["."] = {
       flag = {"hidden", "no-ignore", "ignore-case"}  // table allowed here
   },
  ["&"] = {
      flag = "glob", // table not allowed here
      cb = function(input)
          return string.format([[*{%s}*]], input)
      end,
  }

Will update documentation as per the call we take on above discussion.

@pr4th4m
Copy link
Author

pr4th4m commented Dec 26, 2023

@fdschmidt93 thoughts on above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants