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

Problem with parse_expressions() for input with if else #82

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maelle
Copy link
Contributor

@maelle maelle commented Jul 11, 2023

Reading the comments and test I'm not sure what's supposed to happen for this case actually, as it's not the same usage of the curly braces.

What's the expected output?

Why use regular expressions as opposed to parsing code? Then hopefully one could exclude if else statements from the transformations? I'm a bit confused.

@maelle
Copy link
Contributor Author

maelle commented Jul 11, 2023

Even more concerning, match_brackets() returns NULL:

ex <- c("if (TRUE) {", "    message(\"blop\")", 
"} else {", "    message(\"bla\"", "    ))", "}")
autotest:::match_brackets (ex)
#> NULL
autotest:::parse_expressions (ex)
#> Error in if (qts[[i]][1] > 0) {: missing value where TRUE/FALSE needed

Created on 2023-07-11 with reprex v2.0.2

I'm wondering if we didn't notice this before, because manual pages rarely feature logic?!

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (1a4e37d) 78.53% compared to head (3776b01) 78.53%.

❗ Current head 3776b01 differs from pull request most recent head 7b06938. Consider uploading reports for the commit 7b06938 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #82   +/-   ##
=======================================
  Coverage   78.53%   78.53%           
=======================================
  Files          29       29           
  Lines        4304     4304           
=======================================
  Hits         3380     3380           
  Misses        924      924           
Impacted Files Coverage Δ
R/text-parsing-fns.R 80.60% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@maelle
Copy link
Contributor Author

maelle commented Jul 11, 2023

@mpadge
Copy link
Member

mpadge commented Jul 19, 2023

Thanks @maelle, but note that all of these hand-parsing routines have been completely removed in current dev version in the typetracer branch. Unsure of current timetable for merging that, but once done it will completely change the way autotest works, to use typetracer instead of all this brittle regex code.

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.

2 participants