Skip to content

Fixes to noecho parsing #18

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

Closed
wants to merge 7 commits into from
Closed

Fixes to noecho parsing #18

wants to merge 7 commits into from

Conversation

hugetim
Copy link
Contributor

@hugetim hugetim commented Oct 11, 2022

Related to #16

Known issues remaining:
quietly and capture blocks should not prepend noisily to commands within the block

(I'm starting to think basing the noecho code on program blocks (#15) would have been easier than the quietly-noisily approach, after all.)

@hugetim
Copy link
Contributor Author

hugetim commented Oct 11, 2022

Tracking when a Stata command is inside a quietly or capture block is more complex than tracking in_program because {} can be nested, whereas the different types of programs (program, mata, python) cannot. So doing it right requires matching open and close brackets.

That is why, at this point, I suspect reviving #15 with the benefit of the new helpers for addressing #delimit and program definitions (separating them out and running them separately) may end up simpler. But we may continue bumping up against other Stata features, in either case.

@ticoneva
Copy link
Owner

I agree it's probably a good idea to use the PR15 method unless a #delimit or program define (and its abbreivation) is detected.

@hugetim
Copy link
Contributor Author

hugetim commented Oct 12, 2022

For future reference, here's a notebook with the manual tests this PR was trying to address (with the results shown for the latest 'master' commit, 0898830, with configuration echo = None).
pystata-kernel manual tests.zip

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