fmt: a regression was introduced in #22025 (vfmt struct init)

[larpon]

Describe the bug

I'm not sure if the follwing vfmt struct init align changes in #22025 was intentional, but in vab a one-liner field: if {} else {} was "unrolled" resulting in a situation where I had to run v fmt -w . twice (See this diff https://github.com/vlang/vab/pull/287/files#diff-d7aec8d71af6b80eee9d9768f16fd94d842270b868b008bf268679456c13df84R520-R527)

The code in question went from:

    deploy_opt := android.DeployOptions{
        verbosity: opt.verbosity
        format: format
        // keystore: keystore
        activity_name: opt.activity_name
        work_dir: opt.work_dir
        v_flags: opt.v_flags
        device_id: opt.device_id
        deploy_file: opt.output
        kill_adb: os.getenv('VAB_KILL_ADB') != ''
        clear_device_log: opt.clear_device_log
        device_log: opt.device_log || opt.device_log_raw
        log_mode: if opt.device_log_raw { android.LogMode.raw } else { android.LogMode.filtered }
        log_tags: log_tags
        run: run
    }

to:

    deploy_opt := android.DeployOptions{
        verbosity: opt.verbosity
        format:    format
        // keystore: keystore
        activity_name:    opt.activity_name
        work_dir:         opt.work_dir
        v_flags:          opt.v_flags
        device_id:        opt.device_id
        deploy_file:      opt.output
        kill_adb:         os.getenv('VAB_KILL_ADB') != ''
        clear_device_log: opt.clear_device_log
        device_log:       opt.device_log || opt.device_log_raw
        log_mode:         if opt.device_log_raw {
            android.LogMode.raw
        } else {
            android.LogMode.filtered
        }
        log_tags: log_tags
        run:      run
    }

Reproduction Steps

It can be seen locally by checking out vlang/vab on master and run v fmt -w cli/options.v

Expected Behavior

I'd argue that the output is prettier if the if {} else {} was kept on one line. Alternatively if it should be "unrolled" it should not take two runs of v fmt -w ..

    deploy_opt := android.DeployOptions{
        verbosity: opt.verbosity
        format:    format
        // keystore: keystore
        activity_name:    opt.activity_name
        work_dir:         opt.work_dir
        v_flags:          opt.v_flags
        device_id:        opt.device_id
        deploy_file:      opt.output
        kill_adb:         os.getenv('VAB_KILL_ADB') != ''
        clear_device_log: opt.clear_device_log
        device_log:       opt.device_log || opt.device_log_raw
        log_mode:         if opt.device_log_raw { android.LogMode.raw } else { android.LogMode.filtered }
        log_tags:         log_tags
        run:              run
    }

Current Behavior

    deploy_opt := android.DeployOptions{
        verbosity: opt.verbosity
        format:    format
        // keystore: keystore
        activity_name:    opt.activity_name
        work_dir:         opt.work_dir
        v_flags:          opt.v_flags
        device_id:        opt.device_id
        deploy_file:      opt.output
        kill_adb:         os.getenv('VAB_KILL_ADB') != ''
        clear_device_log: opt.clear_device_log
        device_log:       opt.device_log || opt.device_log_raw
        log_mode:         if opt.device_log_raw {
            android.LogMode.raw
        } else {
            android.LogMode.filtered
        }
        log_tags: log_tags
        run:      run
    }

Possible Solution

No response

Additional Information/Context

No response

V version

V 0.4.7 c51d30b

Environment details (OS name and version, etc.)

Linux

[!NOTE] You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote. Other reactions and those to comments will not be taken into account.

[JalonSolov]

At the least, it would've looked "better" (subjectively, of course) if it had been formatted as

deploy_opt := android.DeployOptions{
        verbosity: opt.verbosity
        format:    format
        // keystore: keystore
        activity_name:    opt.activity_name
        work_dir:         opt.work_dir
        v_flags:          opt.v_flags
        device_id:        opt.device_id
        deploy_file:      opt.output
        kill_adb:         os.getenv('VAB_KILL_ADB') != ''
        clear_device_log: opt.clear_device_log
        device_log:       opt.device_log || opt.device_log_raw
        log_mode:         if opt.device_log_raw {
                    android.LogMode.raw
                  } else {
                    android.LogMode.filtered
                  }
        log_tags: log_tags
        run:      run
    }

In other words, choose the right-hand alignment point as the base for everything on the right side.

[larpon]

Agreed. No matter the style, it is a bug that vfmt needs to be run twice 🙂 (first run creates incorrect formatted code)

[spytheman]

Yes, ideally, running vfmt should produce stable output, no matter what the source is.