Skip to content

Quoted strings/here-strings issues #167

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
msftrncs opened this issue Apr 28, 2019 · 2 comments · Fixed by #173
Closed

Quoted strings/here-strings issues #167

msftrncs opened this issue Apr 28, 2019 · 2 comments · Fixed by #173

Comments

@msftrncs
Copy link
Contributor

msftrncs commented Apr 28, 2019

Adding to issue #143,

  1. Both double-quote and single-quote here-strings fail to tolerate trailing space on the opening line. Users might only notice the @ not highlighted, because regular quoted string still fires, but if any further quotes are used in the here-string, they will cause highlighting issues.
  2. Single-quoted strings fail to highlight, if immediately following a backtick'd single-quote. Example:
    echo `''hello there'
    This statement outputs 'hello there.
    (a) This statement highlights incorrectly because the begin regex incorrectly requires a match of a single-quote to which is not preceded by single-quote. This is flawed because the previous single-quote could be backticked. In reality the check for a prior single-quote is unneeded.
    (b) Likewise the end regex requires matching a single quote not followed by another single quote, but this requirement can be eliminated by using the TextMate applyEndPatternLast property, setting it to true.
  3. (a) Double quoted strings do not exhibit a similar behavior because the begin regex already checked for a backtick before the double-quote that cannot be before the real double-quote. But that logic is flawed. The backtick may have been backtick'd. However, it works, because, the need to check for a preceding double-quote is unnecessary.
    (b) The end match for double-quoted strings can benefit from the same property as the single-quoted string.
  4. Both double-quote and single-quote here-strings fail to scope the begin and end tokens as punctuation.

NOTE: these are relatively minor issues overall.

msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue Apr 28, 2019
permits whitespace to follow here-string start token.
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue Apr 28, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue Apr 28, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue Apr 28, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue Apr 28, 2019
@msftrncs
Copy link
Contributor Author

Just added item 4. These items could use to have tests added when they are fixed (1,2a and 4).

Would need to determine what that scope should be for item 4.

While I have commits for fixes/improvements, I'll hold off on a PR until given direction on combining #143, this issue (all 4 parts), and maybe #141. For instances, items 2 and 3 make #141 easier to implement. Do I combine them (but each committed separately) because they cover the same lines (or very close) or do I create PR's for each but incrementally creating each new branch from the previous one?

@msftrncs
Copy link
Contributor Author

msftrncs commented Apr 29, 2019

Thinking about it some more,

Each one will affect different lines. EDIT: #141 would require changing the same lines as both #143 and this issue, part 1.
Each one could use tests added. A test for this issue's part 2a/3a would be included with the PR for #141.

Maybe its best in the end to just put them all in one PR?

msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
permits whitespace to follow here-string start token.
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
adds punctuation scope to start/end of here-string
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
permits whitespace to follow here-string start token.
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
msftrncs added a commit to msftrncs/EditorSyntax that referenced this issue May 9, 2019
adds punctuation scope to start/end of here-string
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 a pull request may close this issue.

1 participant