Skip to content

Complicated regex in ValidatePattern breaks the grammar #132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vors opened this issue Jul 29, 2015 · 14 comments
Closed

Complicated regex in ValidatePattern breaks the grammar #132

vors opened this issue Jul 29, 2015 · 14 comments

Comments

@vors
Copy link
Member

vors commented Jul 29, 2015

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 vors added this to the 2.2.0 milestone Jul 29, 2015
@vors
Copy link
Member Author

vors commented Feb 12, 2016

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 [email protected]
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
#132 (comment)
.

@vors
Copy link
Member Author

vors commented Feb 12, 2016

Oh, apparently another comment was removed

@daviwil
Copy link

daviwil commented Feb 12, 2016

@godeater this is the right place :)

@godeater
Copy link

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
Copy link

daviwil commented Feb 12, 2016

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
Copy link
Member Author

vors commented Feb 12, 2016

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

@godeater
Copy link

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
Copy link
Member Author

vors commented Feb 12, 2016

@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
Copy link

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

@godeater
Copy link

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

@ScriptingDad
Copy link

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
Copy link
Member Author

vors commented Dec 26, 2017

@godeater hey sorry for a very long reply time: completely slipped out of my radar. It looks like you sent #145 , right?

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

@vors
Copy link
Member Author

vors commented Dec 26, 2017

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

@ScriptingDad
Copy link

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

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

No branches or pull requests

4 participants