Skip to content

Improved FunctionList's NSIS parser#2462

Closed
negrutiu wants to merge 1 commit intonotepad-plus-plus:masterfrom
negrutiu:functionlist
Closed

Improved FunctionList's NSIS parser#2462
negrutiu wants to merge 1 commit intonotepad-plus-plus:masterfrom
negrutiu:functionlist

Conversation

@negrutiu
Copy link
Copy Markdown

@negrutiu negrutiu commented Oct 21, 2016

Improved FunctionList's NSIS syntax:

  • Added !echo keyword

  • Detect functions with comments following the function name:

    Function Test1  ; Comment1
    FunctionEnd
    
    Function Test2  /* Comment2 */
    FunctionEnd
    

- Added "!echo" keyword
- Detect functions with comments following the function name:

  Function Test1  ; Comment1
  FunctionEnd

  Function Test2  /* Comment2 */
  FunctionEnd
@negrutiu negrutiu changed the title Improved FunctionList's NSIS syntax: Improved FunctionList's NSIS parser Oct 21, 2016
Copy link
Copy Markdown
Contributor

@MAPJe71 MAPJe71 left a comment

Choose a reason for hiding this comment

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

Current parsers are not ideal. I'm working on improving the FunctionList parsers.
Thanks for the suggestions but I won't integrate them.

id ="nsis_syntax"
displayName="NSIS Syntax"
commentExpr="(?s:/\*.*?\*/)|(?m-s:[#;].*?$)"
commentExpr="(?s:^[\t\x20]*/\*.*?\*/)|(?m-s:^[\t\x20]*[#;].*?$)"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The at-start-of-line (^) and optional leading blanks ([\t\x20]*) don't improve the comment detection but rather make it worse i.e. it prevents detecting trailing comment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's your decision and I respect it...
You're right of course, when speaking about the multi-line /* */ comments.
But for the single-line comments ; and #, my suggestion could still have some value...
The final expression would be: commentExpr="(?s:/\*.*?\*/)|(?m-s:^[\t\x20]*[#;].*?$)"
Just a thought... :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could still have some value

What value exactly?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Here's a perfectly valid NSIS script: Test.nsi.txt
No function is identified with the default parser rules.

With the changes I've suggested (almost) all functions are detected. SubTest4 is of course incorrectly displayed, but still, it becomes much easier to navigate inside large scripts.

Nevertheless, not a critical problem. I'm waiting for the improved parsers...

>
<function
mainExpr="^[\t\x20]*(!macro|Function|Section|SectionGroup)[\t\x20]+[^\r\n]*$"
mainExpr="^[\t\x20]*(!macro|Function|Section|SectionGroup|!echo)[\t\x20]+[^\r\n]*$"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The !echo will list text messages in the FunctionList tree.
That might be your personal preference but is not something to include in the list of official parsers.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Being a 10+ years NSIS enthusiast I realized that !echo keyword is hardly used in the NSIS community...
Since FunctionList doesn't have "grouping capabilities", I used !echo ----------- to insert separators between blocks of code. I gained more visibility in the function list.
But yes, it's more of a personal preference...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

doesn't have "grouping capabilities"

Yes it does (somewhat) with classRange. I intend to use it for SectionGroup.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That sounds great

@negrutiu negrutiu closed this Oct 22, 2016
@MAPJe71 MAPJe71 mentioned this pull request Nov 24, 2016
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