From 0f40f33dfd790af399be6f5376bab1b53796462b Mon Sep 17 00:00:00 2001 From: Tyler Wilding Date: Sat, 18 May 2024 21:50:34 -0400 Subject: [PATCH] formatter: more improvements and support for things like lambdas --- common/formatter/formatter.cpp | 7 ++ common/formatter/rules/formatting_rules.cpp | 22 +++-- common/formatter/rules/rule_config.cpp | 72 ++++++++++++---- scripts/gsrc/format-gsrc-file.py | 82 +++++++++++-------- .../formatter/corpus/conditions.test.gc | 15 +++- .../formatter/corpus/constant-pairs.test.gc | 57 ++++++++++++- .../common/formatter/corpus/functions.test.gc | 22 ++++- test/common/formatter/corpus/indent.test.gc | 40 ++++++++- test/common/formatter/corpus/lambda.test.gc | 48 +++++++++++ 9 files changed, 302 insertions(+), 63 deletions(-) create mode 100644 test/common/formatter/corpus/lambda.test.gc diff --git a/common/formatter/formatter.cpp b/common/formatter/formatter.cpp index 0af789ed27..7a55f56e13 100644 --- a/common/formatter/formatter.cpp +++ b/common/formatter/formatter.cpp @@ -340,6 +340,13 @@ std::vector apply_formatting(const FormatterTreeNode& curr_node, form_lines.at(form_lines.size() - 1) += fmt::format(" {}", line); } i++; + // We have to handle hang-consolidation here or else it will never be reached above! + if (i == (int)curr_node.refs.size() - 1 && form_lines.size() > 1 && + (curr_node.formatting_config.hang_forms || + curr_node.formatting_config.combine_first_two_lines)) { + form_lines.at(0) += fmt::format(" {}", form_lines.at(1)); + form_lines.erase(form_lines.begin() + 1); + } } if (!curr_node.metadata.is_top_level && next_ref.metadata.node_type == "comment" && (i + 1) == (int)curr_node.refs.size()) { diff --git a/common/formatter/rules/formatting_rules.cpp b/common/formatter/rules/formatting_rules.cpp index f1a5277d16..c5068198ec 100644 --- a/common/formatter/rules/formatting_rules.cpp +++ b/common/formatter/rules/formatting_rules.cpp @@ -133,12 +133,24 @@ bool is_element_second_in_constant_pair_new(const FormatterTreeNode& prev_node, curr_node.node_prefix.value() == "'") { return true; } - // Constant forms special cases (ie. meters) - if (!curr_node.refs.empty() && - constant_type_forms.find(curr_node.refs.at(0).token_str()) != - constant_type_forms.end()) { - return true; + if (!curr_node.refs.empty()) { + // Constant forms special cases (ie. meters) + if (constant_type_forms.find(curr_node.refs.at(0).token_str()) != constant_type_forms.end()) { + return true; + } + // If they are just a list of symbol names (enum or simple method call) + bool all_symbols = true; + for (const auto& ref : curr_node.refs) { + if (ref.metadata.node_type != "sym_name") { + all_symbols = false; + break; + } + } + if (all_symbols) { + return true; + } } + } return false; } diff --git a/common/formatter/rules/rule_config.cpp b/common/formatter/rules/rule_config.cpp index 499b7818b6..bc64addc95 100644 --- a/common/formatter/rules/rule_config.cpp +++ b/common/formatter/rules/rule_config.cpp @@ -1,5 +1,7 @@ #include "rule_config.h" +#include "common/util/string_util.h" + namespace formatter_rules { namespace config { @@ -27,6 +29,12 @@ static FormFormattingConfig new_inlineable_flow_rule(int start_index, [start_index](const std::vector& curr_lines) { int total_width = 0; for (const auto& line : curr_lines) { + // Check for comments + // TODO - this shows how this isn't really the best strategy but it holds up + if (str_util::contains(line, ";")) { + // Can't inline, there's a comment! + return start_index; + } total_width += line.length(); // an empty line implies a new-line was forced, this is bleeding implementation // details, but fine for now @@ -49,10 +57,8 @@ static FormFormattingConfig new_defstate_rule(int start_index, bool has_constant .inline_until_index = [start_index](const std::vector& /*curr_lines*/) { return start_index; }, .has_constant_pairs = has_constant_pairs}; - // TODO - might be nice to have a function that returns a config based on a given index, instead - // of hardcoding them! - std::vector state_handler_indexes = {4, 6, 8, 10, - 12}; // NOTE - not all of these have to be defined + std::vector state_handler_indexes = {4, 6, 8, 10, + 12, 14}; // NOTE - not all of these have to be defined for (const auto& index : state_handler_indexes) { auto temp_config = std::make_shared(); temp_config->config_set = true; @@ -68,21 +74,46 @@ static FormFormattingConfig new_defstate_rule(int start_index, bool has_constant } static FormFormattingConfig new_defmethod_rule(int start_index, bool has_constant_pairs = false) { - return {.config_set = true, - .hang_forms = false, - .inline_until_index = - [start_index](const std::vector& curr_lines) { - if (curr_lines.size() >= 2 && curr_lines.at(1) == "new") { - // defmethod was changed to omit the type name for everything except the `new` - // method, so special case. - return start_index + 1; - } - return start_index; - }, - .has_constant_pairs = has_constant_pairs}; + // TODO - might be nice to have a function that returns a config based on a given index, instead + // of hardcoding them! + // Right now this only works for non-`new` methods (else we may bleed into the body of a normal + // method) + auto arg_list_config = std::make_shared(); + arg_list_config->force_inline = true; + arg_list_config->hang_forms = false; + FormFormattingConfig cfg = {.config_set = true, + .hang_forms = false, + .inline_until_index = + [start_index](const std::vector& curr_lines) { + if (curr_lines.size() >= 2 && curr_lines.at(1) == "new") { + // defmethod was changed to omit the type name for everything + // except the `new` method, so special case. + return start_index + 1; + } + return start_index; + }, + .has_constant_pairs = has_constant_pairs}; + cfg.index_configs.emplace(2, arg_list_config); + return cfg; } -static FormFormattingConfig new_defnum_rule() { +static FormFormattingConfig new_lambda_rule(int start_index, bool has_constant_pairs = false) { + FormFormattingConfig cfg = {.config_set = true, + .hang_forms = false, + .inline_until_index = + [start_index](const std::vector& curr_lines) { + if (curr_lines.size() >= 2 && curr_lines.at(1) == ":behavior") { + // defmethod was changed to omit the type name for everything + // except the `new` method, so special case. + return start_index + 2; + } + return start_index; + }, + .has_constant_pairs = has_constant_pairs}; + return cfg; +} + +static FormFormattingConfig new_defenum_rule() { auto temp_list_config = std::make_shared(); temp_list_config->force_inline = true; temp_list_config->hang_forms = false; @@ -179,8 +210,9 @@ const std::unordered_map opengoal_form_config {"in-package", new_top_level_inline_form(true)}, {"bundles", new_top_level_inline_form(true)}, {"require", new_top_level_inline_form(true)}, - {"defenum", new_defnum_rule()}, + {"defenum", new_defenum_rule()}, {"defmethod", new_defmethod_rule(3)}, + {"lambda", new_lambda_rule(2)}, {"deftype", new_deftype_rule(3, {3, 4, 5, 6})}, {"defun", new_flow_rule(3)}, {"defun-debug", new_flow_rule(3)}, @@ -188,8 +220,12 @@ const std::unordered_map opengoal_form_config {"if", new_inlineable_flow_rule(2)}, {"#if", new_inlineable_flow_rule(2)}, {"define", new_permissive_flow_rule()}, + {"defmethod-mips2c", new_permissive_flow_rule()}, {"define-extern", new_permissive_flow_rule()}, {"defmacro", new_flow_rule(3)}, + {"defskelgroup", new_flow_rule(2, true)}, + {"defpartgroup", new_flow_rule(2, true)}, + {"defpart", new_flow_rule(2, true)}, {"defstate", new_defstate_rule(3, true)}, {"behavior", new_flow_rule(2)}, {"dotimes", new_flow_rule(2)}, diff --git a/scripts/gsrc/format-gsrc-file.py b/scripts/gsrc/format-gsrc-file.py index 4d5640185c..3a58c06b60 100644 --- a/scripts/gsrc/format-gsrc-file.py +++ b/scripts/gsrc/format-gsrc-file.py @@ -10,46 +10,56 @@ # f.write(json.dumps(json_data, indent=2)) import json +import math import subprocess -import sys +from colorama import just_fix_windows_console, Fore, Back, Style -apply_status = None -if len(sys.argv) > 1: - apply_status = sys.argv[1] +just_fix_windows_console() -with open("./scripts/gsrc/format-jak1.json", "r") as f: - formatting_progress = json.load(f) +def format_the_file(apply_status): + with open("./scripts/gsrc/format-jak1.json", "r") as f: + formatting_progress = json.load(f) -# find the next file -curr_file = None -curr_file_index = 0 -go_to_next = False -open_file_in_vscode = False -if apply_status == "next": - go_to_next = True - open_file_in_vscode = True + # find the next file + curr_file = None + go_to_next = False + open_file_in_vscode = False + if apply_status == "next": + go_to_next = True + open_file_in_vscode = True -for index, file in enumerate(formatting_progress): - if file['status'] == 'not-formatted': - if go_to_next: - print(f"Marking {file['path']} as formatted") - formatting_progress[index]['status'] = 'formatted' - go_to_next = False + for index, file in enumerate(formatting_progress): + if file['status'] == 'not-formatted': + if go_to_next: + print(f"Marking {file['path']} as formatted") + print(f"{Fore.GREEN} {(index / len(formatting_progress)) * 100:.3f}% {Fore.RESET} Completed") + formatting_progress[index]['status'] = 'formatted' + go_to_next = False + else: + curr_file = file['path'] + if open_file_in_vscode: + subprocess.run(["C:\\Users\\xtvas\\AppData\\Local\\Programs\\Microsoft VS Code\\bin\\code.cmd", file['path']]) + break + + # format it + print(f"Formatting {curr_file}") + subprocess.run(["./out/build/Debug/bin/formatter", "--write", "--file", curr_file]) + + # save status + if apply_status is not None and (apply_status == "skip" or apply_status == "next"): + # TODO - add skip support back if i ever want to use it + with open("./scripts/gsrc/format-jak1.json", "w") as f: + f.write(json.dumps(formatting_progress, indent=2)) + + subprocess.run(["git", "diff", "--shortstat", "origin/master"]) + +def main(): + while True: + user_input = input(Fore.CYAN + "Type 'n' to proceed to the next file or just hit enter to re-format the current file: " + Fore.RESET) + if user_input == 'n': + format_the_file("next") else: - curr_file = file['path'] - curr_file_index = index - if open_file_in_vscode: - subprocess.run(["C:\\Users\\xtvas\\AppData\\Local\\Programs\\Microsoft VS Code\\bin\\code.cmd", file['path']]) - break + format_the_file(None) -# format it -print(f"Formatting {curr_file}") -subprocess.run(["./out/build/Debug/bin/formatter", "--write", "--file", curr_file]) - -# save status -if apply_status is not None and (apply_status == "skip" or apply_status == "next"): - # TODO - add skip support back if i ever want to use it - with open("./scripts/gsrc/format-jak1.json", "w") as f: - f.write(json.dumps(formatting_progress, indent=2)) - -subprocess.run(["git", "diff", "--shortstat", "origin/master"]) \ No newline at end of file +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/test/common/formatter/corpus/conditions.test.gc b/test/common/formatter/corpus/conditions.test.gc index c3bde25eab..da305ee98c 100644 --- a/test/common/formatter/corpus/conditions.test.gc +++ b/test/common/formatter/corpus/conditions.test.gc @@ -74,4 +74,17 @@ Consolidation of if 2 (if (or (not s5-1) (= (-> s5-1 name) 'default)) (login this) ;; not part of level load, just normal login. - ) \ No newline at end of file + ) + +=== +Consolidation of if with comment +=== + +(if (= (-> obj prim-id) arg0) ;; it's us! + (return obj) + ) + +--- + +(if (= (-> obj prim-id) arg0) ;; it's us! + (return obj)) \ No newline at end of file diff --git a/test/common/formatter/corpus/constant-pairs.test.gc b/test/common/formatter/corpus/constant-pairs.test.gc index d83169f6f2..c6a5305518 100644 --- a/test/common/formatter/corpus/constant-pairs.test.gc +++ b/test/common/formatter/corpus/constant-pairs.test.gc @@ -181,4 +181,59 @@ Special case forms :down-move-to-pitch-ratio-in-air 0.5 :up-move-to-pitch-on-ground 0.9 :down-move-to-pitch-on-ground 0.9 - :pitch-off-blend 0.5)) \ No newline at end of file + :pitch-off-blend 0.5)) + +=== +Enums +=== + +(define *warp-jump-mods* + (new 'static + 'surface + :name 'jump + :turnv 273066.66 + :turnvv 1820444.5 + :tiltv 32768.0 + :tiltvv 131072.0 + :transv-max 65536.0 + :target-speed 65536.0 + :slip-factor 1.0 + :slide-factor 1.0 + :slope-up-factor 1.0 + :slope-down-factor 1.0 + :slope-slip-angle 1.0 + :impact-fric 1.0 + :bend-factor 1.0 + :bend-speed 1.0 + :alignv 1.0 + :slope-up-traction 1.0 + :align-speed 1.0 + :mode 'air + :flags + (surface-flags always-rotate-toward-transv))) + +--- + +(define *warp-jump-mods* + (new 'static + 'surface + :name 'jump + :turnv 273066.66 + :turnvv 1820444.5 + :tiltv 32768.0 + :tiltvv 131072.0 + :transv-max 65536.0 + :target-speed 65536.0 + :slip-factor 1.0 + :slide-factor 1.0 + :slope-up-factor 1.0 + :slope-down-factor 1.0 + :slope-slip-angle 1.0 + :impact-fric 1.0 + :bend-factor 1.0 + :bend-speed 1.0 + :alignv 1.0 + :slope-up-traction 1.0 + :align-speed 1.0 + :mode 'air + :flags (surface-flags always-rotate-toward-transv))) \ No newline at end of file diff --git a/test/common/formatter/corpus/functions.test.gc b/test/common/formatter/corpus/functions.test.gc index c8ca1dabeb..0d0ba7fc42 100644 --- a/test/common/formatter/corpus/functions.test.gc +++ b/test/common/formatter/corpus/functions.test.gc @@ -96,4 +96,24 @@ Methods - new (defmethod new align-control ((allocation symbol) (type-to-make type) (proc process-drawable)) "Create a new align-control." - (+ 1 1)) \ No newline at end of file + (+ 1 1)) + +=== +Methods - long arg list +=== + +(defmethod fill-and-probe-using-line-sphere ((obj collide-cache) (arg0 vector) + (arg1 vector) + (arg2 float) + (arg3 collide-kind) + (arg4 process) + (arg5 collide-tri-result) + (arg6 pat-surface)) + (fill-using-line-sphere obj arg0 arg1 arg2 arg3 (the-as process-drawable arg4) arg6) + (probe-using-line-sphere obj arg0 arg1 arg2 arg3 arg5 arg6)) + +--- + +(defmethod fill-and-probe-using-line-sphere ((obj collide-cache) (arg0 vector) (arg1 vector) (arg2 float) (arg3 collide-kind) (arg4 process) (arg5 collide-tri-result) (arg6 pat-surface)) + (fill-using-line-sphere obj arg0 arg1 arg2 arg3 (the-as process-drawable arg4) arg6) + (probe-using-line-sphere obj arg0 arg1 arg2 arg3 arg5 arg6)) \ No newline at end of file diff --git a/test/common/formatter/corpus/indent.test.gc b/test/common/formatter/corpus/indent.test.gc index 2e23faa1a5..c6d6c872f9 100644 --- a/test/common/formatter/corpus/indent.test.gc +++ b/test/common/formatter/corpus/indent.test.gc @@ -147,4 +147,42 @@ Long array creation ))) (dotimes (i 16) (set! (-> (scratchpad-object terrain-context) work foreground joint-work frm-jmp-table i) - (the-as (function none) (-> arr i))))) \ No newline at end of file + (the-as (function none) (-> arr i))))) + +=== +!Nested function call +=== + +(set! (-> this attack-event) + (the-as symbol ((the-as + (function res-lump symbol symbol float structure (pointer res-tag) pointer object) + (method-of-type res-lump get-property-struct) + ) + (-> this entity) + 'attack-event + 'interp + -1000000000.0 + 'dark-eco-pool + (the-as (pointer res-tag) #f) + *res-static-buf* + ) + ) + ) + +--- + +(set! (-> this attack-event) + (the-as symbol ((the-as + (function res-lump symbol symbol float structure (pointer res-tag) pointer object) + (method-of-type res-lump get-property-struct) + ) + (-> this entity) + 'attack-event + 'interp + -1000000000.0 + 'dark-eco-pool + (the-as (pointer res-tag) #f) + *res-static-buf* + ) + ) + ) \ No newline at end of file diff --git a/test/common/formatter/corpus/lambda.test.gc b/test/common/formatter/corpus/lambda.test.gc new file mode 100644 index 0000000000..f72e1f6627 --- /dev/null +++ b/test/common/formatter/corpus/lambda.test.gc @@ -0,0 +1,48 @@ +=== +With behavior tag +=== + +(lambda :behavior + target + () + (println "something")) + +--- + +(lambda :behavior target () + (println "something")) + +=== +Without behavior tag +=== + +(lambda () + (println "something")) + +--- + +(lambda () + (println "something")) + +=== +Too long +=== + +(lambda ((arg0 part-tracker)) + (let ((s5-0 (handle->process (-> arg0 userdata)))) (when s5-0 (let* ((v1-4 (handle->process (-> (the-as eco-collectable s5-0) pickup-handle))) (a2-0 (cond ((not v1-4) (-> arg0 root trans)) ((= (-> v1-4 type) target) (vector<-cspace! (new 'stack-no-clear 'vector) (-> (the-as target v1-4) node-list data 5))) (else (-> (the-as target v1-4) control trans))))) (vector-lerp! (-> arg0 root trans) (-> arg0 offset) a2-0 (/ (the float (- (current-time) (-> arg0 start-time))) (the float (-> (the-as eco-collectable s5-0) collect-effect-time)))))))) + +--- + +(lambda ((arg0 part-tracker)) + (let ((s5-0 (handle->process (-> arg0 userdata)))) + (when s5-0 + (let* ((v1-4 (handle->process (-> (the-as eco-collectable s5-0) pickup-handle))) + (a2-0 (cond + ((not v1-4) (-> arg0 root trans)) + ((= (-> v1-4 type) target) (vector<-cspace! (new 'stack-no-clear 'vector) (-> (the-as target v1-4) node-list data 5))) + (else (-> (the-as target v1-4) control trans))))) + (vector-lerp! (-> arg0 root trans) + (-> arg0 offset) + a2-0 + (/ (the float (- (current-time) (-> arg0 start-time))) + (the float (-> (the-as eco-collectable s5-0) collect-effect-time)))))))) \ No newline at end of file