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

[BUG] Formating comment lines #110

Closed
brentlblair opened this issue Jun 12, 2020 · 19 comments
Closed

[BUG] Formating comment lines #110

brentlblair opened this issue Jun 12, 2020 · 19 comments
Assignees
Labels
bug Something isn't working resolved Issue resolved or enhancement added
Milestone

Comments

@brentlblair
Copy link
Contributor

Not sure if this is considered a bug or the intended outcome. Either way, it's a minor issue. I noticed that the format document also indents the comments lines to the configured code margin. I think the comment lines should either be ignored or moved to zero margin. In my opinion, it makes the program harder to read.

This is what I think the formatted document should look like:

Code_jWPIqwin7S

This is what the format document looks like today:
Code_WYBF7daESF

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

Looking at the code that handles formatting, comment lines should already be ignored. I use a zero margin in my own user settings, so haven't noticed what you're reporting.

I'll take a look at what might be causing this "regression".

@dthiot
Copy link
Contributor

dthiot commented Jun 12, 2020 via email

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

@dthiot That's an interesting idea. If it's something you would like to see, would you please enter it as a new enhancement request? Otherwise, it may get lost in the comments thread of this issue.

@kpowick kpowick self-assigned this Jun 12, 2020
@kpowick kpowick added the bug Something isn't working label Jun 12, 2020
@dthiot
Copy link
Contributor

dthiot commented Jun 12, 2020 via email

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

Thanks, @dthiot !

I get what you're saying and hope I didn't give the impression that I was trying to dismiss your idea or put you off. It makes sense to ask, right? Why bother creating a new request if nobody is really interested?

I was just trying to preemptively keep the comments on this issue from drifting into a discussion not specifically on topic for it. For example, participants agree that your idea is a good one, then start discussing how it should be implemented, options available, naming conventions, etc.

@kpowick kpowick added the resolved Issue resolved or enhancement added label Jun 12, 2020
@kpowick kpowick added this to the v2.0.9 milestone Jun 12, 2020
@pschellenbach
Copy link

While on the issue of formatting comments, wED has two comment settings: one for '*' comments and one for '!' comments. Each can be set to indent none, margin or code level. No special formatting is provided for REM statements.

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

@brentlblair This issue is resolved. Comment lines are now ignored when it comes to indentation. This actually used to be the case, and I'm still trying to figure out where this "regression" crept in.

Maybe @itsxallwater can produce a hotfix for this? I'm not sure if an entirely new release is needed. I suppose I should figure out how to build a VSIX file.

@itsxallwater
Copy link
Member

I think I'd like this feature toggled to be honest. I tend to keep comments indented with code. I could be in the minority (or alone altogether) but this feels like the kind of thing that we'd be better served to make an option.

@itsxallwater
Copy link
Member

Re: building a vsix I've got you covered @kpowick! Check this: #96 (comment)

@brentlblair
Copy link
Contributor Author

brentlblair commented Jun 12, 2020 via email

@dthiot
Copy link
Contributor

dthiot commented Jun 12, 2020 via email

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

@itsxallwater I like my comments aligned with code as well, and the latest commit to develop does not change your comment indentation, which was the problem seen by @brentlblair .

@dthiot
Copy link
Contributor

dthiot commented Jun 12, 2020 via email

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

Re: building a vsix I've got you covered @kpowick! Check this: #96 (comment)

@itsxallwater Thanks for that. I've got a vsix for the extension (still at 2.0.8) that @brentlblair can use to update his extension. Two questions:

  1. Should I have changed the version number for this vsix "hotfix"? It's still 2.0.8

  2. Where can I make this vsix file available? I have an on-line storage provider from which I can create a download link.

@itsxallwater
Copy link
Member

itsxallwater commented Jun 12, 2020

You can attach files here in your comment. I've found that's become the most handy, the link is shareable beyond just the comment and it keeps things close to home. Now that the vsix is tiny it works well!

Edit: regarding the version number, yeah ideally you'd bump it up apologies for that.

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

@brentlblair Attached is a vsix file for a patched version of the extension 2.0.8.

To Install

  1. Download the attached zip file and extract the file mvbasic-2.0.8.patched.vsix
  2. Open VS Code
  3. Open the Command Palette cmd+shift+P or menu/view/Command Palette
  4. Type vsix to find the option Extensions: Install from VSIX...
  5. Select that option.
  6. From the file dialogue that opens, select the file mvbasic-2.0.8.patched.vsix and click on the "install" button.

You'll have to do a document reload for the change to take effect.

Please let me know how it goes.

mvbasic-2.0.8.patched.zip

@kpowick
Copy link
Member

kpowick commented Jun 12, 2020

Edit: regarding the version number, yeah ideally you'd bump it up apologies for that.

Didn't see your edit until after posting the vsix. As the formatting option was the only change, it's probably ok just to leave it as is for now, don't you think?

@brentlblair
Copy link
Contributor Author

brentlblair commented Jun 12, 2020 via email

itsxallwater added a commit that referenced this issue Sep 1, 2020
* RegEx fix for labels with leading spaces.

* 98-Issue linting with multiple levels of quoting

* RegEx improvement for quoted strings

* Replace parenthesis and quoted strings with empty ones.
- Removed repetitive Trim()s
- Removed unnecessary RegEx tests
- Simplified line split on semicolon due to above changes.
- Results: Faster execution. Better coverage for split on semicolon

* - Cleanup lines[] array at start so repeated test/replace not needed
- Removed unnecessary trim()s
- Removed boolean equality tests. if(x.test == true) vs if(x.test)
- Fixed GOTO/GOSUB when followed by more than one space GOTO <s><s> 100

* RegEx fix-ups to support recent changes

* Corrected comment

* Linter RegEx cleanup

* FOR/NEXT tracking improvements and updated error messages
- Nested For/Next statements now validated
- NEXT with no variable is now allowed
- One line per error: Removed extraneous, repeated diagnostic messages
- Reworded / Clarified some error messages for consistency

* Minor code clean-up

* Replace char-at-a-time split on ; with split(";")

* Maintain original character spacing for Intellisense error markers
- RegEx improvements and consistency between client and server
- Improved getWord() function

* Corrected indentation for ELSE END with trailing comment (issue #45)

* Corrects Label error with GOTO/GOSUB. Issue #99
- Also now allows dotted numeric labels such as GOTO 100.5

* Revert "Corrects Label error with GOTO/GOSUB. Issue #99"

This reverts commit 68b0166.

* Inspect TM Scopes->Inspect Editor Token and Scopes

* Fix error processing customWordPath when last
line is blank. May relate to #24.

* Changed GOTO/GOSUB scope jumping message from Error to Warning

* Numerous updates related to For/Next and Label tracking
- Issue #45: Alignment of If/Else
- Issue #96: Allow periods in labels
- Issue #99: Corrected Label error with GOTO/GOSUB.
- Issue #104: Corrected For/Next with leading label
- Issue #105: Changed GOTO scope jumping message from Error to Warning
- RegEx: Label comments to include "REM, *, and !"
- NEXT can be used without FOR variable.

* Shipping 2.0.7

* Re-introduce accidentally deleted /doc files

* Fixes issue 107: FOR/NEXT error when no whitespace after FOR variable.

* Shipping 2.0.8

* Issue #110: Do not change indentation of comment lines

* Issue #111: Added user setting to control indentation of comment lines.

* Bump elliptic from 6.5.2 to 6.5.3 (#119)

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.2 to 6.5.3.
- [Release notes](https://github.com/indutny/elliptic/releases)
- [Commits](indutny/elliptic@v6.5.2...v6.5.3)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update Extension Guide.md

Updated AccuTerm connector and host program links.

* Feature request 113 (#121)

* Update package.json

Added MVBasic.customFunctionPath

* Update server.ts

added functionality to Load CustomFunction definition

* Add files via upload

* Update Extension Guide.md

added documentation for Custom functions and subroutines 6.8

* Update server.ts

Removed `data: functions[i].index,` for now since Microsoft has not responded yet.  It works without it, and it's no longer in the Microsoft API documentation.

* - Fix issue with startLoop/endLoop in server code parser
- Clean up unused variable in server
- Expand docs on new customFunctions feature

* Shipping 2.0.9

Co-authored-by: Kevin Powick <[email protected]>
Co-authored-by: Ian McGowan <[email protected]>
Co-authored-by: Peter Schellenbach <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Brent Blair <[email protected]>
@itsxallwater
Copy link
Member

2.0.9 released! Closing issue.

itsxallwater added a commit that referenced this issue Sep 2, 2020
* RegEx fix for labels with leading spaces.

* 98-Issue linting with multiple levels of quoting

* RegEx improvement for quoted strings

* Replace parenthesis and quoted strings with empty ones.
- Removed repetitive Trim()s
- Removed unnecessary RegEx tests
- Simplified line split on semicolon due to above changes.
- Results: Faster execution. Better coverage for split on semicolon

* - Cleanup lines[] array at start so repeated test/replace not needed
- Removed unnecessary trim()s
- Removed boolean equality tests. if(x.test == true) vs if(x.test)
- Fixed GOTO/GOSUB when followed by more than one space GOTO <s><s> 100

* RegEx fix-ups to support recent changes

* Corrected comment

* Linter RegEx cleanup

* FOR/NEXT tracking improvements and updated error messages
- Nested For/Next statements now validated
- NEXT with no variable is now allowed
- One line per error: Removed extraneous, repeated diagnostic messages
- Reworded / Clarified some error messages for consistency

* Minor code clean-up

* Replace char-at-a-time split on ; with split(";")

* Maintain original character spacing for Intellisense error markers
- RegEx improvements and consistency between client and server
- Improved getWord() function

* Corrected indentation for ELSE END with trailing comment (issue #45)

* Corrects Label error with GOTO/GOSUB. Issue #99
- Also now allows dotted numeric labels such as GOTO 100.5

* Revert "Corrects Label error with GOTO/GOSUB. Issue #99"

This reverts commit 68b0166.

* Inspect TM Scopes->Inspect Editor Token and Scopes

* Fix error processing customWordPath when last
line is blank. May relate to #24.

* Changed GOTO/GOSUB scope jumping message from Error to Warning

* Numerous updates related to For/Next and Label tracking
- Issue #45: Alignment of If/Else
- Issue #96: Allow periods in labels
- Issue #99: Corrected Label error with GOTO/GOSUB.
- Issue #104: Corrected For/Next with leading label
- Issue #105: Changed GOTO scope jumping message from Error to Warning
- RegEx: Label comments to include "REM, *, and !"
- NEXT can be used without FOR variable.

* Shipping 2.0.7

* Re-introduce accidentally deleted /doc files

* Fixes issue 107: FOR/NEXT error when no whitespace after FOR variable.

* Shipping 2.0.8

* Issue #110: Do not change indentation of comment lines

* Issue #111: Added user setting to control indentation of comment lines.

* Bump elliptic from 6.5.2 to 6.5.3 (#119)

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.2 to 6.5.3.
- [Release notes](https://github.com/indutny/elliptic/releases)
- [Commits](indutny/elliptic@v6.5.2...v6.5.3)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update Extension Guide.md

Updated AccuTerm connector and host program links.

* Feature request 113 (#121)

* Update package.json

Added MVBasic.customFunctionPath

* Update server.ts

added functionality to Load CustomFunction definition

* Add files via upload

* Update Extension Guide.md

added documentation for Custom functions and subroutines 6.8

* Update server.ts

Removed `data: functions[i].index,` for now since Microsoft has not responded yet.  It works without it, and it's no longer in the Microsoft API documentation.

* - Fix issue with startLoop/endLoop in server code parser
- Clean up unused variable in server
- Expand docs on new customFunctions feature

* Shipping 2.0.9

* Update server.ts

It looks like I left the line `customFunctionPath = settings.MVBasic.customFunctionPath;` out of the merge.

* Shipping v2.0.10

Co-authored-by: Kevin Powick <[email protected]>
Co-authored-by: Ian McGowan <[email protected]>
Co-authored-by: Peter Schellenbach <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Brent Blair <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resolved Issue resolved or enhancement added
Projects
None yet
Development

No branches or pull requests

5 participants