SublimeText / PowerShell

Support for the MS PowerShell programming language.
MIT License
312 stars 80 forks source link

Complicated regex in ValidatePattern breaks the grammar #132

Closed vors closed 6 years ago

vors commented 9 years ago
Function Get-Domain
{
    [CmdletBinding()]
    param(
        [ValidatePattern('^(?=^.{1,254}$)(^(?:(?!\d+\.)[a-zA-Z0-9_\-]{1,63}\.?)+(?:[a-zA-Z]{2,})$)')]
        [Parameter(ValueFromPipeline = $true)] # this line should not be highlighted as a string
        [string]$domain
    )
}
vors commented 8 years ago

Yes, this is the right place. Atom, VSCode and GitHub uses the same grammar.

On Fri, Feb 12, 2016 at 6:07 AM, Bryan Childs notifications@github.com wrote:

I have a fix for this, but I'm not sure where it should be sent - it seems various different editors use TextMate's language definition for Powershell, but I've never used TextMate (and can't, being as how I abhor Macs), so I can't text the fix there...

— Reply to this email directly or view it on GitHub https://github.com/SublimeText/PowerShell/issues/132#issuecomment-183344563 .

vors commented 8 years ago

Oh, apparently another comment was removed

daviwil commented 8 years ago

@godeater this is the right place :)

godeater commented 8 years ago

I removed the comment I put earlier as I found that the new grammar I had, although it didn't wreck the syntax highlighting through the rest of the buffer as the current one does, now only highlights on the first "variable=value" in a [Parameter( block. I'm trying to work out if it's worth submitting it still as the lesser of two evils, or keep plugging away to see if I can fix the regex properly.

daviwil commented 8 years ago

Go ahead and fix it properly if you want to. Definitely doesn't make sense to pull it in yet if it breaks other highlighting.

vors commented 8 years ago

This regex became pretty complicated, good luck. You can share preview of your results with https://github-lightshow.herokuapp.com/

godeater commented 8 years ago

s/want to/can/ =/

After I noticed that problem, I kept looking at it all afternoon without much success.

Take a look at these and see which you think would be best to use.

With current grammar: before

With new grammar: after

vors commented 8 years ago

@godeater send a PR with a reference to this issue, we can take a look together.

It always takes few hours to switch context to tmLanguage syntax, but I feel there were no releases for quite some time and GitHub's highlighting is out-of-date.

godeater commented 8 years ago

I've sent a pull request, but not sure if I've done it right or not.

godeater commented 8 years ago

I presume the decision was taken that this wasn't good enough?

ScriptingDad commented 6 years ago

Was this ever resolved? I seem to be having the same issue at present and it's making it difficult to switch to atom for powershell.

vors commented 6 years ago

@godeater hey sorry for a very long reply time: completely slipped out of my radar. It looks like you sent https://github.com/SublimeText/PowerShell/pull/145 , right?

I think all maintainers just dropped ball here at the time, not that it was not good enough. Sorry for that.

vors commented 6 years ago

The plan was to move highlighting development to https://github.com/PowerShell/EditorSyntax but it stalled a little bit too.

ScriptingDad commented 6 years ago

Thanks for the update. Seems there are a few bugs related to syntax highlighting in there. I'll see how they're progressing.