Skip to content

Conversation

@yhx-12243
Copy link
Contributor

The original code

if (q && q.startsWith('(')) {
return
} else {
return parse('(' + q + ')')
}
use the logic that, if q starts with ( then regard q as a whole atom. But this may be fail on something like (T1 | T2) & T3, as following:

(T1 | T2) & T3 fails

The simplest fix is just modify the peggy syntax, adding a new item named Wrapped (atom) just wrap an formula with single “(”“)”, e.g., (T1), and it works.

(T1 | T2) & T3 works

@yhx-12243 yhx-12243 marked this pull request as draft June 25, 2025 13:10
@yhx-12243 yhx-12243 marked this pull request as ready for review June 25, 2025 14:13
@yhx-12243
Copy link
Contributor Author

@StevenClontz Do you have time to review this?

@StevenClontz
Copy link
Member

I need to check in with @jamesdabbs to unstick the reviewing queue on this repository.

Copy link
Member

@jamesdabbs jamesdabbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏻

@StevenClontz - we're closing on a house and in last rounds of interviews. I'm optimistic about and looking forward to surfacing here before too long.

@StevenClontz
Copy link
Member

I'm pretty flexible MWF this semester (except for the past month which has been hectic). Let's plan to synchronize when you come up for air next.

@StevenClontz StevenClontz merged commit 81bc07f into pi-base:main Sep 21, 2025
1 check passed
@yhx-12243 yhx-12243 deleted the formula-parse branch September 21, 2025 23:40
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.

3 participants