fix call breaks - #229
Conversation
Code Coverage SummaryDiff against mainResults for commit: 7cc307e Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 12 suites 3s ⏱️ Results for commit 7cc307e. ♻️ This comment has been updated with latest results. |
Unit Test Performance DifferenceAdditional test case details
Results for commit 07d75b2 ♻️ This comment has been updated with latest results. |
|
Thanks for writing an extensive PR description. Very useful for people reviewing it.
Not a big deal in the end. I remember how many things you needed to deal with to get the parser to work and keep the code as-is. The current state on this PR is still better than what we had on main before last Friday, so this is still a huge improvement. Will review on my own today |
|
Hey @gogonzo this is great! I left one comment that I think is within our reach |
closes #228
Core issue is that
split_codeinterprets data fromparseDatawhich returns exact index of the end of the call. Based on the information of the call-break, we decided also to associate a call-terminator to the current call. For example, if call ends at line: 1 and col: 7 (a <- 11has 7 characters) we can also add\nto this call by changing index to line: 1; col: 7 + 2. This is how we do it on main. Thanks to this we keep exact formatting in@codeslot and we are sure thateval_codeandget_codewould return the same result.Problem arises with
;as it is not a two character element, and indexing could cause a failure. In thesplit_code/get_codewe somehow managed this by adding some extra "cleaners" and still keep the eval_code-get_code consistent somehow.However,
tm_g_pp_vitalsmodule does something nasty. Module produces very long call and deparse must have done something not easy to handle bysplit_code. See the #228 to see howsplit_callfails.Solution: Because end of the call is detected properly and sometimes there is a problem with detecting where the next call begins, I decided to start the next call at the end of the previous and remove
;or\nfrom the beginning. This solution results in@codeto not store call terminators andget_codecombines calls by\n. In other words;is converted into\nalways.