search

spmeesseman / vscode-taskexplorer

Run and Manage Tasks for Visual Studio Code
Other
141 stars 30 forks source link

Change Dashed Groups to be configurable #104

Closed richarddavenport closed 4 years ago

richarddavenport commented 4 years ago

I love the dashed groups, but some of my projects use a color (":").

This PR:

  1. Changes all references to the Group Dashed configuration to Group With Separator.
  2. Adds a Group Separator configuration.
ray007 commented 4 years ago

Very nice, I like it. But since you're already touching this: why not make it "list of characters" or go straight to "configurable regular expression"? For most cases going with /\W/ would work nicely I think...

richarddavenport commented 4 years ago

It actually does work that way. It's just using the split method, which can handle those cases: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split I think the documentation could just be updated.

ray007 commented 4 years ago

While the split method of String does take a RegExp as first parameter, your groupSeparator constant seems to be of type string.

richarddavenport commented 4 years ago

Hmm, good point. The vscode api doesn't look like it supports regex out of the box. We would need to detect if it was a regex. Do you think that you have a use case for that? I think it would be nice if @spmeesseman could approve this first and then could enhance it later.

spmeesseman commented 4 years ago

hi, sorry for the delay, ill get to this PR as soon as I have a chance

spmeesseman commented 4 years ago

oh and thank you btw

ray007 commented 4 years ago

@richarddavenport detecting whether or not to use a regex should not be hard.

If groupSeparator is not a single char, make it a regexp. If it starts and ends with / use the part in between as is, otherwise use it as character selection:

rx = new RegExp(`[${groupSeparator}]`)

Or add another checkbox for any non-word char as setting, since I suspect the most probably usecase will be /\W/.

richarddavenport commented 4 years ago

@ray007 I completely understand, but I'd really like to get this in now, as it stand now, and then enhance it. My personal use case is to just need to it to split on a colon, which seems like your original request on 102.

spmeesseman commented 4 years ago

@ray007 - and of course, when i finallly get a chance to pull your change in that ive been wanting to do for awhile, the Azure Pipelines is down and theres an Advisory that they had an "accident" and to watch the advisory boards for more info. Figures. In any case, finally got this in. Pipeline will run at some point today i assume... thanks for your contrib!!!

richarddavenport commented 4 years ago

@spmeesseman thanks for merging!

spmeesseman commented 4 years ago

:tada: This PR is included in version 1.27.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: