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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

vfmt: fix const comments formatting #20005

Merged
merged 6 commits into from Nov 27, 2023
Merged

vfmt: fix const comments formatting #20005

merged 6 commits into from Nov 27, 2023

Conversation

StunxFS
Copy link
Contributor

@StunxFS StunxFS commented Nov 27, 2023

Fix #20000.

$ cat main.v
const (
	abc = 1 // abc
	def = 2 // def
	ghi = 3 // ghi
)

fn main() {}
$ v fmt main.v
const abc = 1 // abc
const def = 2 // def
const ghi = 3 // ghi

fn main() {}

馃[deprecated] Generated by Copilot at 0a83026

This pull request enhances the parsing and formatting of constant blocks in V by fixing a syntax error, adding conditional statements, and updating the test files. It affects the files vlib/v/parser/parser.v and vlib/v/fmt/tests/consts_input.vv and vlib/v/fmt/tests/consts_expected.vv.

馃[deprecated] Generated by Copilot at 0a83026

  • Preserve and format inline comments of constant blocks in V (link, link, link, link)
  • Add a block of constants with inline comments to consts_input.vv and consts_expected.vv to test the formatting of constant blocks with different delimiters and comments (link, link)
  • Modify parse_const_decl in parser.v to append any comments that follow the closing delimiter of a constant block to the end_comments array, and to reset the end_comments array after each constant block (link, link)
  • Fix a syntax error in consts_input.vv by removing a trailing comma from the multiline_const array (link)

@spytheman
Copy link
Member

That would not work, vfmt has to be changed too, so that it generates source code that will not have to be formatted 2 times in a row.

@spytheman
Copy link
Member

I have a patch for that locally, but I can not push it here, github gives me a permission denied error.

@spytheman
Copy link
Member

image

if it will help, here it is as a text diff too:

commit 35bce9c47f91257a0a7c7e7d2fe25f3af674c2f6 (HEAD -> fix_vfmt)
Author: Delyan Angelov <delian66@gmail.com>
Date:   Mon Nov 27 13:32:40 2023 +0200

    fix the `consts_expected.vv` case, make sure to generate source that is invariant over further `v fmt` runs

diff --git vlib/v/fmt/fmt.v vlib/v/fmt/fmt.v
index 0089de380..6994c3bce 100644
--- vlib/v/fmt/fmt.v
+++ vlib/v/fmt/fmt.v
@@ -913,7 +913,7 @@ pub fn (mut f Fmt) const_decl(node ast.ConstDecl) {
        } else {
                ast.Node(ast.NodeError{})
        }
-       for _, field in node.fields {
+       for fidx, field in node.fields {
                if field.comments.len > 0 {
                        if f.should_insert_newline_before_node(ast.Expr(field.comments[0]), prev_field) {
                                f.writeln('')
@@ -936,7 +936,8 @@ pub fn (mut f Fmt) const_decl(node ast.ConstDecl) {
                f.write('= ')
                f.expr(field.expr)
                f.comments(field.end_comments, same_line: true)
-               if node.is_block && field.end_comments.len == 0 {
+               if node.is_block && fidx < node.fields.len - 1 && node.fields.len > 1 {
+                       // old style grouped consts, converted to the new style ungrouped const
                        f.writeln('')
                } else {
                        // Write out single line comments after const expr if present
diff --git vlib/v/fmt/tests/consts_expected.vv vlib/v/fmt/tests/consts_expected.vv
index 699ebbb41..e9a1d1341 100644
--- vlib/v/fmt/tests/consts_expected.vv
+++ vlib/v/fmt/tests/consts_expected.vv
@@ -26,7 +26,9 @@ const i_am_a_very_long_constant_name_so_i_stand_alone_and_my_length_is_over_90_c
 pub const i_am_pub_const = true
 
 const abc = 1 // abc
+
 const def = 2 // def
+
 const ghi = 3 // ghi
 
 fn main() {

@spytheman
Copy link
Member

spytheman commented Nov 27, 2023

You can test it locally by doing:

echo ">>>>>>>>>>>>>>>>>>>>>>>>  first pass:" && v fmt in.v && echo ">>>>>>>>>>>>>>>>>>>>>>>> second pass:" && v fmt in.v |v fmt

where in.v is:

const (
        abc = 1 // abc
        def = 2 // def
        ghi = 3 // ghi
)

@StunxFS
Copy link
Contributor Author

StunxFS commented Nov 27, 2023

You can test it locally by doing:

echo ">>>>>>>>>>>>>>>>>>>>>>>>  first pass:" && v fmt in.v && echo ">>>>>>>>>>>>>>>>>>>>>>>> second pass:" && v fmt in.v |v fmt

where in.v is:

const (
        abc = 1 // abc
        def = 2 // def
        ghi = 3 // ghi
)

Ok, thank you 馃榿
Locally I had no problems when running the tests 馃

@spytheman
Copy link
Member

for me, without the vfmt patch, either vlib/v/fmt/fmt_keep_test.v failed, or vlib/v/fmt/fmt_test.v failed

spytheman added a commit to vlang/go2v that referenced this pull request Nov 27, 2023
@spytheman spytheman merged commit 2964855 into vlang:master Nov 27, 2023
53 of 54 checks passed
@StunxFS StunxFS deleted the fix_vfmt branch November 27, 2023 14:31
@lmptg
Copy link

lmptg commented Nov 27, 2023

@StunxFS fantastic work! Thanks a lot 馃挴

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 this pull request may close these issues.

vfmt's recent automatic const scope unwrapping ruins valuable trailing comments in vlang/sdl
3 participants