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

fmt: implement wrapping function's super long arguments (fix #15545, #21643) #21780

Closed
wants to merge 2 commits into from

Conversation

yuyi98
Copy link
Member

@yuyi98 yuyi98 commented Jul 1, 2024

This PR implement wrapping function's super long arguments (fix #15545, fix #21643).

  • Implement wrapping function's super long arguments.

  • Add test.

  • vlib\v\fmt\tests\fn_with_long_args_keep.vv

fn process_keyholder1(super_long_argument1 string, super_long_argument2 string, super_long_argument3 string,
	super_long_argument4 string, super_long_argument5 string) string {
	return ''
}

fn process_keyholder2(super_long_argument1 string, super_long_argument2 string, super_long_argument3 string,
	super_long_argument4 string, super_long_argument5 string, super_long_argument6 string, super_long_argument7 string,
	super_long_argument8 string) string {
	return ''
}

fn main() {}

Comment on lines -128 to +129
fn (mut ts TestSession) append_message_with_duration(kind MessageKind, msg string, d time.Duration, mtc MessageThreadContext) {
fn (mut ts TestSession) append_message_with_duration(kind MessageKind, msg string, d time.Duration,
mtc MessageThreadContext) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is shorter than 140 characters (127), why is it still wrapped?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is in accordance with the non-last parameter length more than 90.

Comment on lines 525 to 526
fn (mut tm DynamicTemplateManager) create_template_cache_and_display(tcs CacheRequest, last_template_mod i64,
unique_time i64, file_path string, tmpl_name string, cache_delay_expiration i64, placeholders &map[string]DtmMultiTypeMap, current_content_checksum string, tmpl_type TemplateType) string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that got wrapped, but the second line is also very long

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was not considered that there would be such a long parameter, I'll fix it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fn (mut tm DynamicTemplateManager) create_template_cache_and_display(tcs CacheRequest, last_template_mod i64,
	unique_time i64, file_path string, tmpl_name string, cache_delay_expiration i64, placeholders &map[string]DtmMultiTypeMap,
	current_content_checksum string, tmpl_type TemplateType) string {

@JalonSolov
Copy link
Contributor

If it's going to wrap arguments because of a too-long line, I'd almost rather see it add a newline after every argument, and line them all up.

This would mean

fn process_keyholder1(super_long_argument1 string, super_long_argument2 string, super_long_argument3 string,
	super_long_argument4 string, super_long_argument5 string) string {
	return ''
}

fn process_keyholder2(super_long_argument1 string, super_long_argument2 string, super_long_argument3 string,
	super_long_argument4 string, super_long_argument5 string, super_long_argument6 string, super_long_argument7 string,
	super_long_argument8 string) string {
	return ''
}

would be turned into

fn process_keyholder1(super_long_argument1 string,
	super_long_argument2 string,
	super_long_argument3 string,
	super_long_argument4 string,
	super_long_argument5 string) string {
	return ''
}

fn process_keyholder2(super_long_argument1 string,
	super_long_argument2 string,
	super_long_argument3 string,
	super_long_argument4 string,
	super_long_argument5 string,
	super_long_argument6 string,
	super_long_argument7 string,
	super_long_argument8 string) string {
	return ''
}

which, to me, is much more readable than the multiple lines of arbitrary numbers of arguments. Just compare what you see above.

@yuyi98 yuyi98 closed this Jul 1, 2024
@yuyi98 yuyi98 deleted the fmt_fn_long_args branch July 1, 2024 14:28
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.

Long function signatures should break vfmt unwrapps long, multiline func signatures to a single line
3 participants