mvextensions / mvbasic

MultiValue Basic extension for Visual Studio Code
MIT License
30 stars 16 forks source link

[BUG] Incorrect Comment highlighting #23

Closed andrewcole50 closed 5 years ago

andrewcole50 commented 5 years ago

Describe the bug Valid comments are not always highlighted correctly

To Reproduce Steps to reproduce the behavior: Any comment that is preceded by a label (both numerical and textual) as well as a comment without a character between the * and the new line.

Expected behavior Expect to have the asterisk and characters after it highlighted as a comment.

Screenshots comments

Versions of applicable software Windows 10 VS Code. Connected to Jbase (CentOS 7) directory via SFTP. MVON version 1.96.0

Code samples and/or reproduceable test cases I did some basic searching through the source files and I think changing the regex for the comments from .+ to . would solve the asterisk without any following characters. Some suggested regex for the other situations are: `^\S+:\s+(*.)|^[0-9]+\s+(*.*)`

Additional context Willing to help solve/test for this issue.

itsxallwater commented 5 years ago

Hey @andrewcole50! Thanks for the great write up.

I've tested your scenario against the jBASE definitions that are being prepared in another ticket so this is not a like-for-like, but I am mentioning because I think it will help further the conversation. I also expanded the scenario to include some same-line comments after semi-colons and to include ! comments.

image

Here's the code in text form if you would like to copy/paste:

    * Multiplication
    i = 10
    i = i * 7
    i = i * 7 ;* Comment after semi-colon
    i = i * 7 ;! Comment after semi-colon    
* Line Comment
! Line Comment
    * Line Comment with Preceding white Space
    ! Line Comment with Preceding white Space
    *
    !
100 * Numerical label followed by Comment
100 ! Numerical label followed by Comment
A.Label: * Text label followed by comment
A.Label: ! Text label followed by comment

So, what does it all mean? Definitively, the regex could be improved upon as you've noted. Secondly, as this grows we'll want to pay attention to the syntax files for other language variants but that's noted just as a thing to keep in mind. All of that said, I'd love for you to take a stab at the change and test, so I'm going to post some instructions on cloning the repo through running it in debug mode wherein you can attempt to change and test these sorts of things.

  1. git clone https://github.com/mvextensions/mvbasic.git
  2. Open that new local folder in VS Code
  3. Go to the Terminal menu and select New Terminal
  4. At the command prompt, type npm install and press enter
  5. When that finishes, type npm run compile and press enter

At this point, you've now cloned, installed and compiled the extension locally. To debug, it's easiest to grab debug.zip and add its two files, launch.json and tasks.json to your .vscode folder. Then, when you press F5 to debug or go to the Debug menu and select Start Debugging you'll notice that debugs a profile called "Launch Client". That will build and launch a new version of VS Code with the extension compiled and loaded in. You'll then want to go back to the parent VS Code instance and change the debug drop down from "Launch Client" to "Attach to Server" and press F5 again (or Start Debugging again). That will connect a debugger to the language server as well. Then you'll be able to debug activity in the new VS Code instance back inside the parent instance, for any client-facing activity or any language server-facing activity.

andrewcole50 commented 5 years ago

So I think I've got this debugging working, however none of the syntax highlighting is working. Where are those settings at so I can test the comments?

itsxallwater commented 5 years ago

Are you modifying the existing MvLanguage.json and mvon.tmLanguage.json files or did you introduce new ones? If new, you'll want to search out and replace the references to both of those files with yours. The former is set in server/src/server.ts, the latter is set in the package.json files.

If you haven't done so since making your mods, you'll want to take care to run npm run compile again.

andrewcole50 commented 5 years ago

I've got it now. Do you need to assign the bug to me in order for me to perform a push? Here's the results of my modifications: comments-fixed

andrewcole50 commented 5 years ago

I'm not sure assigning really did anything. I think I submitted my changes...I'm still rather new to git...

itsxallwater commented 5 years ago

Yep sorry, just got off a call with the team. As an external contributor to this what you'll want to do is fork the repository into your account, so you'll end up with something https://github.com/andrewcole50/mvbasic. You would clone that to your dev machine, make your changes, commit and push them back up to your repo, then come here and create a Pull Request. When you do that, you can choose the option to do a Pull Request from a fork in order to select your version.

It essentially has to do with repo security and access. Check out https://help.github.com/en/articles/creating-a-pull-request-from-a-fork from the GitHub folks it's a pretty good write up. If the steps sound confusing, trust me, it's easier than it sounds/looks.

itsxallwater commented 5 years ago

Note: Resolved by PR #25

itsxallwater commented 5 years ago

PR accepted and merged, thanks @andrewcole50!