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

feat: Add an option to remove parethesis arround table call #329

Closed
shadmansaleh opened this issue Jan 2, 2022 · 4 comments · Fixed by #330
Closed

feat: Add an option to remove parethesis arround table call #329

shadmansaleh opened this issue Jan 2, 2022 · 4 comments · Fixed by #330
Labels
enhancement New feature or request

Comments

@shadmansaleh
Copy link
Contributor

shadmansaleh commented Jan 2, 2022

@JohnnyMorganz firstly great work on stylua . Thanks for making it happen.

I like having having function call parenthesis on only string call like

fn("asdf")

But not when only table is used for function call . like

fn { x = 'a', y = 'b' }

Two layers of parenthesis on

fn({x='a', y='b'})

seems unnecessary and no parens when only string is used gets weird time to time

require 'xyz' '32'

Currently it's only possible to either have parenthesis on both cases or on none . Would've really liked if there was a middle ground .

Is it possible to add an option to only remove parenthesis on table calls ?

It's farely easy to add

diff --git a/src/formatters/functions.rs b/src/formatters/functions.rs
index 1a356e9..1101f4f 100644
--- a/src/formatters/functions.rs
+++ b/src/formatters/functions.rs
@@ -107,7 +107,7 @@ pub fn format_function_args(
             arguments,
         } => {
             // Handle config where parentheses are omitted, and there is only one argument
-            if ctx.config().no_call_parentheses
+            if (ctx.config().no_call_parentheses || ctx.config().no_table_call_parentheses)
                 && arguments.len() == 1
                 && !matches!(call_next_node, FunctionCallNextNode::ObscureWithoutParens)
             {
@@ -115,12 +115,16 @@ pub fn format_function_args(
                 if let Expression::Value { value, .. } = argument {
                     match &**value {
                         Value::String(token_reference) => {
-                            return format_function_args(
-                                ctx,
-                                &FunctionArgs::String(token_reference.to_owned()),
-                                shape,
-                                call_next_node,
-                            );
+                            if ctx.config().no_call_parentheses {
+                                return format_function_args(
+                                    ctx,
+                                    &FunctionArgs::String(token_reference.to_owned()),
+                                    shape,
+                                    call_next_node,
+                                );
+                            } else {
+                                ();
+                            }
                         }
                         Value::TableConstructor(table_constructor) => {
                             return format_function_args(
@@ -506,7 +510,7 @@ pub fn format_function_args(
         }
 
         FunctionArgs::TableConstructor(table_constructor) => {
-            if ctx.config().no_call_parentheses
+            if (ctx.config().no_call_parentheses || ctx.config().no_table_call_parentheses)
                 && !matches!(call_next_node, FunctionCallNextNode::ObscureWithoutParens)
             {
                 let table_constructor = format_table_constructor(ctx, table_constructor, shape)
diff --git a/src/lib.rs b/src/lib.rs
index aa1510d..e7b77be 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -94,6 +94,8 @@ pub struct Config {
     /// Whether to omit parentheses around function calls which take a single string literal or table.
     /// This is added for adoption reasons only, and is not recommended for new work.
     no_call_parentheses: bool,
+    /// Whether to omit parentheses around function calls which take only a single table.
+    no_table_call_parentheses: bool,
 }
 
 impl Config {
@@ -149,6 +151,13 @@ impl Config {
             ..self
         }
     }
+    /// Returns a new config with the given value for [`no_table_call_parentheses`]
+    pub fn with_no_table_call_parentheses(self, no_table_call_parentheses: bool) -> Self {
+        Self {
+            no_table_call_parentheses,
+            ..self
+        }
+    }
 }
 
 impl Default for Config {
@@ -160,6 +169,7 @@ impl Default for Config {
             indent_width: 4,
             quote_style: QuoteStyle::default(),
             no_call_parentheses: false,
+            no_table_call_parentheses: false,
         }
     }
 }

If you're in favor of the idea I can open a pr .

@JohnnyMorganz
Copy link
Owner

firstly great work on stylua . Thanks for making it happen.

Thank you! Hope its working well for you

Given that we already have a no_call_parentheses option, and all this does is meet halfway and line it up a bit more with the base defaults, I think this is reasonable to add - sounds like it might help in some cases too!

The only thing I would say is that I would prefer a more general option combining the two separate boolean options no_call_parentheses / no_table_call_parentheses, given that they are related. Maybe something more along the lines of an enum option call_parentheses with values "Always" (default), "None" (replacing no_call_parentheses), and "NoTable" (or "StringOnly"?) for no_table_call_parentheses.

Would need to handle the no_call_parentheses option for backwards compatibility, and deprecate it in favour of the enum variant.

Feel free to open up a PR if you want, or I can look into it when I've got some time

@JohnnyMorganz JohnnyMorganz added the enhancement New feature or request label Jan 2, 2022
@shadmansaleh
Copy link
Contributor Author

shadmansaleh commented Jan 3, 2022

Thanks for your reply.

Hope its working well for you

Indeed . It's working really well.

The only thing I would say is that I would prefer a more general option combining the two separate boolean options no_call_parentheses / no_table_call_parentheses, given that they are related. Maybe something more along the lines of an enum option call_parentheses with values "Always" (default), "None" (replacing no_call_parentheses), and "NoTable" (or "StringOnly"?) for no_table_call_parentheses.

That sounds like an even better idea .I'll look into implementing that.

@clason
Copy link

clason commented Jan 3, 2022

History repeating ;) #133 (comment)

@JohnnyMorganz
Copy link
Owner

History repeating ;) #133 (comment)

Heh, looking back at it I guess diluting is a con, but I think this option is reasonable and doesn't really have a big impact. And it reduces the deviation from the defaults (always parentheses), which is something I like!

I'll probably leave strings for now unless there really is a significant demand for it, and I think the enum option leaves us room for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants