Skip to content
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

Batch syntax highlighting doesn't work properly with expansion modifiers #4

Closed
riQQ opened this issue May 13, 2021 · 10 comments
Closed
Labels
batch Caused by the batch lexer bug Something isn't working

Comments

@riQQ
Copy link
Contributor

riQQ commented May 13, 2021

Example: echo %~dp09
Only %~dp0 should be highlighted as a batch identifier instead of %~dp09.

Notepad++ Screenshot (the screenshot was made with an older version, but it's still reproducable with Notepad++ 7.9.5)
Bash syntax highlighting

This was already reported at notepad-plus-plus/notepad-plus-plus#63.

@nyamatongwe
Copy link
Member

It would help to include a link to documentation of the expansion modifier syntax.

@ArkadiuszMichalski
Copy link
Contributor

Only %~dp0 should be highlighted as a batch identifier instead of %~dp09.

I see different editors do it differently:

  • VSC ~dp0
  • Sublime Text %~dp0
  • all based on Scintilla %~dp09

Regardless, 9 should not be taken into account, because it's just string, like echo %~dp0hello or echo %~dp0 hello.

It may be useful:
https://ss64.com/nt/syntax-args.html

Running for /? in cmd will return some information:

In addition, substitution of FOR variable references has been enhanced.
You can now use the following optional syntax:

%~I         - expands %I removing any surrounding quotes (")
%~fI        - expands %I to a fully qualified path name
%~dI        - expands %I to a drive letter only
%~pI        - expands %I to a path only
%~nI        - expands %I to a file name only
%~xI        - expands %I to a file extension only
%~sI        - expanded path contains short names only
%~aI        - expands %I to file attributes of file
%~tI        - expands %I to date/time of file
%~zI        - expands %I to size of file
%~$PATH:I   - searches the directories listed in the PATH
               environment variable and expands %I to the
               fully qualified name of the first one found.
               If the environment variable name is not
               defined or the file is not found by the
               search, then this modifier expands to the
               empty string

The modifiers can be combined to get compound results:

%~dpI       - expands %I to a drive letter and path only
%~nxI       - expands %I to a file name and extension only
%~fsI       - expands %I to a full path name with short names only
%~dp$PATH:I - searches the directories listed in the PATH
               environment variable for %I and expands to the
               drive letter and path of the first one found.
%~ftzaI     - expands %I to a DIR like output line

In the above examples %I and PATH can be replaced by other valid
values.  The %~ syntax is terminated by a valid FOR variable name.
Picking upper case variable names like %I makes it more readable and
avoids confusion with the modifiers, which are not case sensitive.

@nyamatongwe
Copy link
Member

The code that implements this is around line 404 of LexBatch.cxx. Changing it is complicated by the need to update several variables to retreat back past additional characters to the digit.

@nyamatongwe nyamatongwe added bug Something isn't working batch Caused by the batch lexer labels May 15, 2021
riQQ added a commit to riQQ/lexilla that referenced this issue May 15, 2021
Expanded arguments have a single digit at the end.

Fixes GitHub issue ScintillaOrg#4.
nyamatongwe pushed a commit that referenced this issue May 15, 2021
Expanded arguments have a single digit at the end. Fixes GitHub issue #4.
Based on #5 with braced moved to match current code, change log added,
and invalid cppcheck warning suppressed.
@nyamatongwe
Copy link
Member

Pull request committed with minor changes.

Credited to "riQQ". If you want a different name in the credits then tell me.

@riQQ
Copy link
Contributor Author

riQQ commented May 16, 2021

That's fine, thanks.

@lifenjoiner
Copy link
Contributor

Came here for 1 more special case:

for /F "tokens=1,3,4 delims=:" %%a in ("!inioff!") do (
set "inioff=%%~a"
set "base=%%~b"
set /A "size=%%~c" )

ppt
" is also colorized. The %%~ type variable ending recognition needs to be improved if there is " (and other special characters).
It can be observed in (tested):

lexilla/lexers/LexBatch.cxx

Lines 400 to 450 in e9bf225

// Check for Expanded Argument (%~...) / Variable (%%~...)
// Expanded Argument: %~[<path-operators>]<single digit>
// Expanded Variable: %%~[<path-operators>]<single identifier character>
// Path operators are exclusively alphabetic.
// Expanded arguments have a single digit at the end.
// Expanded variables have a single identifier character as variable name.
} else if (((wbl > 1) && (wordBuffer[1] == '~')) ||
((wbl > 2) && (wordBuffer[1] == '%') && (wordBuffer[2] == '~'))) {
// Check for External Command / Program
if (cmdLoc == offset - wbl) {
cmdLoc = offset - (wbl - wbo);
}
bool isArgument = (wordBuffer[1] == '~');
if (isArgument) {
Sci_PositionU expansionStopOffset = 2;
bool isValid = false;
for (; expansionStopOffset < wbl; expansionStopOffset++) {
if (isArgument && Is0To9(wordBuffer[expansionStopOffset])) {
expansionStopOffset++;
isValid = true;
wbo = expansionStopOffset;
// Colorize Expanded Argument
styler.ColourTo(startLine + offset - 1 - (wbl - wbo), SCE_BAT_IDENTIFIER);
break;
}
}
if (!isValid) {
// not a valid expanded argument or variable
styler.ColourTo(startLine + offset - 1 - (wbl - wbo), SCE_BAT_DEFAULT);
}
// Expanded Variable
} else {
// start after ~
wbo = 3;
// Search to end of word for another % (can be a long path)
while ((wbo < wbl) &&
(wordBuffer[wbo] != '%')) {
wbo++;
}
if (wbo > 3) {
// Colorize Expanded Variable
styler.ColourTo(startLine + offset - 1 - (wbl - wbo), SCE_BAT_IDENTIFIER);
} else {
// not a valid expanded argument or variable
styler.ColourTo(startLine + offset - 1 - (wbl - wbo), SCE_BAT_DEFAULT);
}
}
// Reset Offset to re-process remainder of word
offset -= (wbl - wbo);
// Check for Environment Variable (%x...%)
} else if ((wordBuffer[1] != '%') &&

Initial discussion at zufuliu/notepad2#332 (comment) as the item 1.

@lifenjoiner
Copy link
Contributor

@nyamatongwe
Copy link
Member

Committed b39c460 patch with some changes to x.bat.styled and x.bat.folded. The patch was a little strange as it appeared to be making new x.bat.styled.new and x.bat.folded.new files but with just the new text as if they were based on a shortened version of x.bat with just the new examples.

Its perfectly OK to provide a new example file (and its processed forms) just for a bug. For example, test/examples/markdown/Bug2235.md.

When adding to an existing example, run the tester, check the resulting .new files then rename them without the .new (replacing the old .styled/.folded) then include these modifications in the patch.

@lifenjoiner
Copy link
Contributor

OK, got it. I misunderstood the test part, thought the owner will do the final step 😅, as drawn by the popup https://github.com/ScintillaOrg/lexilla/blob/e9bf2252938311d2c75119a59cf776885c5e224e/CONTRIBUTING#L6-L10:

lexilla/CONTRIBUTING

Lines 6 to 10 in e9bf225

Patches should include test cases. Add a test case file in
lexilla/test/examples/<language> and run the test program in
lexilla/test.
The result will be new files with ".styled.new" and ".folded.new"
appended containing lexing or folding results.

Thanks for your effort!

As we use git/hg, changes can be easily traced, may just patching the .styled/.folded files be OK?

@nyamatongwe
Copy link
Member

There's more explanation further on in CONTRIBUTING. Sending patches containing just the additions is good.

@riQQ riQQ closed this as completed May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
batch Caused by the batch lexer bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants