search

idaholab / moose-language-support

MOOSE language support for VSCode
Other
5 stars 4 forks source link

VSCode: FormatDocument deletes !include #24

Closed jmeier closed 1 month ago

jmeier commented 2 months ago

Dear Community,

I have the Moose input file shown below. On "Format Document" (what is done automatically on save), the line "!include ..." will be deleted.

[Mesh]
  [file]
    type = FileMeshGenerator
    file = "suzanne.msh"
  []
[]

!include models/suzanne/suzanne.groups.i

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

[AuxVariables]
  [dummy]
    order = FIRST
  []
[]

[Outputs]
  exodus = true
[]
brandonlangley commented 1 month ago

@grunerjmeier -

This problem should have been fixed and merged into the MOOSE framework on January 24.

How old is the code from your MOOSE app or MOOSE being used your app (not this plugin)?

If it at least includes the 58d0497 commit from January 24, you should not see this problem.

A separate issue where formatting expanded each ${var} was later fixed in MOOSE as well.

That other fix with brace expression formatting was merged into the framework on March 21.

If your MOOSE includes 132e9f8 from March 21, you should not have either formatting issue.

However various other fixes and new features for input support have been added since then.

So it is maybe best to keep your MOOSE up to date with the latest code as much as you can.

jmeier commented 1 month ago

Dear @brandonlangley,

Thanks for your answer!

I update Moose quite often. At the moment I'm in #next on 3b0c2f9c (25.07.2024). Subequently, I have to re-compile my MooseApp. And I still have the problem that "!include" gets deleted. Do I have to compile something else to update the language server (despite my Moose app)?

jmeier commented 1 month ago

Some more info on my setup:

brandonlangley commented 1 month ago

@grunerjmeier -

Yes you are 100% correct about this, sorry about my incorrect suggestion earlier.

I verified the problem with everything up to date, tracked down the cause, and figured out a fix.

This was working once but regressed with other changes, and the fix is just a three line update.

I will add the fix to MOOSE as soon as I am able and will let you know when it has been merged.

Thanks so much for reporting this.

jmeier commented 1 month ago

Dear @brandonlangley,

thank you for taking your time and looking into this! I'm glad a fix is on its way.

I assume you close this issue once the fix is merged.

Jörg

brandonlangley commented 1 month ago

@grunerjmeier -

When it has been merged into MOOSE, I will let you know so you can update and verify it fixes your problem.

Then you can close this issue after you make sure that the fix works for you.

jmeier commented 1 month ago

Dear @brandonlangley,

I've seen that https://github.com/idaholab/moose/pull/28285 got merged. So I updated my copy of the moose git-repo (now including commit 55dca4d), re-compiled my app and re-started my VSCode. Sadly, I still got some issues.

I think it might be connected to the fact, if the file to incude is found or not.

I would be very pleased that the includes are not deleted even if the include target does not (or not yet) exist.

Test 1: include on an existing file in the same folder

This works. The include remains in tact.

Test 2: include on an nonexisting file in the same folder

This does NOT work. The include gets deleted.

Test 3: include on an existing file in a sub-folder

This works. The include remains in tact.

Test 4: include on an nonexisting file in a sub-folder

This does NOT work. The include gets deleted.

Test 5: typo "incluSe"

!incuSe bla.i

gets changed to 

!incuSe = !incuSe bla.i

what gets changed to

!incuSe = !incuSe
bla.i = bla.i

Language server crashing

Also, vscode is complaining, that the language server is crashing too often. This happens, while editing.

The MOOSE Language Server server crashed 5 times in the last 3 minutes. The server will not be restarted. See the output for more information.

VS-Code Output

``` Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK input in flex scanner failed [Info - 7:41:32 AM] Connection to server got closed. Server will restart. true [Error - 7:41:32 AM] Server process exited with code 2. input in flex scanner failed [Info - 7:41:32 AM] Connection to server got closed. Server will restart. true [Error - 7:41:32 AM] Server process exited with code 2. input in flex scanner failed [Info - 7:41:33 AM] Connection to server got closed. Server will restart. true [Error - 7:41:33 AM] Server process exited with code 2. input in flex scanner failed [Info - 7:41:33 AM] Connection to server got closed. Server will restart. true [Error - 7:41:33 AM] Server process exited with code 2. [Error - 7:41:33 AM] Delivering pending changes failed TypeError: this.task is not a function at /home/mjg/.vscode-server/extensions/danielschwen.moose-language-support-1.1.2/out/main.js:35:44419 input in flex scanner failed [Error - 7:41:33 AM] The MOOSE Language Server server crashed 5 times in the last 3 minutes. The server will not be restarted. See the output for more information. [Error - 7:41:33 AM] Server process exited with code 2. ```

brandonlangley commented 1 month ago

@grunerjmeier -

Thank you for all the testing and yes you are correct, the problem is as simple as your original thought.

The formatting is not writing the !include filename.i directive back out if filename.i is not found.

The previous update fixed the issue for existing files but the problem is still there for nonexistent ones.

I see what is going on here and it should be a very simple fix that I will ask @dschwen to review again.

Sorry about the hassle, if you would like to test the fix before it gets merged this time just let me know.

jmeier commented 1 month ago

Dear @brandonlangley,

Thanks for looking into this again and taking your time.

if you would like to test the fix before it gets merged this time just let me know.

If you would like me to test it, I would be happy to do so.

brandonlangley commented 1 month ago

@grunerjmeier -

Earlier, the idaholab/moose#28346 PR was merged into MOOSE to address this situation.

It became clear that it was best to not let malformed input be formatted when you asked about more cases like:

!incluSe bla.i
!include 'bla[c].i'

If an input has malformed structural problems then they must first be fixed before the formatting will work, e.g.,

And these structural problems can be fixed by looking at the syntax error messages in the Problems panel, e.g.,

syntax error, 'type' has a missing or malformed value [Ln 2, Col 8]
syntax error, unexpected invalid token, expecting = or := [Ln 3, Col 7]
syntax error, unexpected end of file, expecting block terminator [Ln 8, Col 1]

So before formatting an input, a user must first make sure it at least structurally sound according to HIT syntax.

This prevents the formatOnSave option from formatting malformed inputs and performing any undefined edits.

So it addresses your cases for bad syntax and missing include files so they will not get automatically formatted.

You should be able to close this if you update your MOOSE with idaholab/moose@474d5f3 to verify this works.

jmeier commented 1 month ago

Dear @brandonlangley,

First of all, thank you very much for these updates!

I find the solution of not formatting syntactically unclear documents much better. It prevents user frustration and data loss. As a nice side effect, the quality of code submitted to Moose as PR may be a bit increased (as a format on save can be performed by default).

I updated my Moose and re-compiled: For me the behaviour is now as you describe. Therefore, I'm happy to close this issuse as "resolved" with this comment.