feat(octal escapes): add support for octal escapes in quoted literals; update parsing, tests, and docs by nlothian · Pull Request #323 · nlothian/Vibe-Prolog

nlothian · 2025-12-05T11:28:11Z

Closes #306

Implement octal escape parsing in PrologTransformer:
- Add _parse_octal_escape to validate and convert octal sequences like \101\ to the corresponding character.
- Raise syntax errors on invalid digits or overflow (>255).
Update string unescaping:
- Process octal escapes before standard escapes in _unescape_string.
- Use a placeholder for handling backslashes to preserve intent.
Adjust grammar/parsing cache:
- Change grammar key construction to cache by the built grammar object (grammar = self._build_grammar(operators); key = grammar) instead of using operators tuple.
Extend string regex to support octal escapes:
- Update STRING rules to allow octal escapes inside both single- and double-quoted literals.
Tests added for octal escapes:
- New TestOctalEscapes with cases for single/double quotes, 1- to 3-digit octals, embedded octals, max value 377 (255), invalid digits, missing terminator, overflow, and empty escapes.
Documentation updates:
- docs/FEATURES.md: note octal escapes in quoted strings.
- docs/GRAMMAR_COMPARISON.md: reflect compatibility and behavior of octal escapes.
- docs/SYNTAX_NOTES.md: document octal escapes in Vibe-Prolog syntax notes.
Overall effect:
- Enables proper parsing and representation of octal escapes within quoted literals, aligns tests and docs with new capability, and fixes grammar caching behavior to avoid inconsistencies.

… and ellipsis ... support; update tokenizer, tests, and docs - Relax dot-as-atom rule: stop rejecting a bare '.' as an atom inside expressions; rely on clause-terminator handling for syntax - Improve tokenizer: introduce ends_with_range_and_dot heuristic to correctly distinguish range expressions (e.g., 1..9) from decimal points - Add tests: new TestDotAndRangeOperator suite covering dot-as-atom, quoted dot, range operator parsing in various contexts (lists, parentheses, CLP(Z)-style constraints, tokenizer behavior) - Update docs: document new features (dot as atom in expressions, range operator .., ellipsis ...) in FEATURES.md - Ensure parsing preserves range/ellipsis patterns in tokenization and AST construction

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…ly-handles-after-numbers' of github.com:nlothian/Vibe-Prolog into 311-parser-fails-on-19-range-syntax-tokenizer-incorrectly-handles-after-numbers

…; update parsing, tests, and docs - Implement octal escape parsing in PrologTransformer: - Add _parse_octal_escape to validate and convert octal sequences like \101\ to the corresponding character. - Raise syntax errors on invalid digits or overflow (>255). - Update string unescaping: - Process octal escapes before standard escapes in _unescape_string. - Use a placeholder for handling backslashes to preserve intent. - Adjust grammar/parsing cache: - Change grammar key construction to cache by the built grammar object (grammar = self._build_grammar(operators); key = grammar) instead of using operators tuple. - Extend string regex to support octal escapes: - Update STRING rules to allow octal escapes inside both single- and double-quoted literals. - Tests added for octal escapes: - New TestOctalEscapes with cases for single/double quotes, 1- to 3-digit octals, embedded octals, max value 377 (255), invalid digits, missing terminator, overflow, and empty escapes. - Documentation updates: - docs/FEATURES.md: note octal escapes in quoted strings. - docs/GRAMMAR_COMPARISON.md: reflect compatibility and behavior of octal escapes. - docs/SYNTAX_NOTES.md: document octal escapes in Vibe-Prolog syntax notes. - Overall effect: - Enables proper parsing and representation of octal escapes within quoted literals, aligns tests and docs with new capability, and fixes grammar caching behavior to avoid inconsistencies.

kilo-code-bot

✅ No Issues Found

5 files reviewed | Confidence: 95% | Recommendation: Merge

Review Details

Files: parser.py (core implementation), test_parser_strings.py (comprehensive tests), docs/FEATURES.md, docs/GRAMMAR_COMPARISON.md, docs/SYNTAX_NOTES.md (documentation updates)

Checked: Security, correctness, error handling, test coverage, documentation

Implementation Notes:

Octal escape parsing correctly validates digits, range (≤255), and termination
Grammar updated to support octal escapes in quoted literals
String unescaping processes octal escapes before standard escapes
Comprehensive test coverage including edge cases (invalid digits, overflow, missing terminator)
Documentation properly updated across all relevant files

gemini-code-assist

Code Review

This pull request introduces excellent support for octal escapes in quoted literals, complete with thorough tests and documentation updates. The change to improve grammar caching is also a valuable fix. I've identified a critical issue in the updated regex for strings that could break existing functionality, along with a few medium-severity suggestions to improve code style, clarity, and test specificity. Overall, this is a great contribution.

gemini-code-assist · 2025-12-05T11:30:11Z

    CHAR_CODE.5: /0'(\\x[0-9a-zA-Z]+\\?|\\\\|\\\\['tnr]|''|[^'\\])'?/ | /[1-9]\d*'.'/

-    STRING: /"([^"\\]|\\.)*"/ | /'(\\.|''|[^'\\])*'/
+    STRING: /"([^"\\]|\\[^"]*)*"/ | /'([^'\\]|\\[^']*|'' )*'/


There appears to be a typo in the updated STRING regex for single-quoted atoms. The '' (with a space) requires a space after a doubled single-quote escape, which is not standard Prolog syntax and will break atom quoting like 'it''s'. This should be '' without the trailing space.

Suggested change

STRING: /"([^"\\]|\\[^"]*)*"/ | /'([^'\\]|\\[^']*|'' )*'/

STRING: /"([^"\\]|\\[^"]*)*"/ | /'([^'\\]|\\[^']*|'')*'/

gemini-code-assist · 2025-12-05T11:30:11Z

+        if not all(c in '01234567' for c in inner):
+            raise PrologThrow(PrologError.syntax_error("invalid octal escape", "escape_sequence/1"))
+        try:
+            value = int(inner, 8)
+            if value > 255:
+                raise PrologThrow(PrologError.syntax_error("octal escape overflow", "escape_sequence/1"))
+            return chr(value)
+        except ValueError:
+            raise PrologThrow(PrologError.syntax_error("invalid octal escape", "escape_sequence/1"))


The explicit check if not all(c in '01234567' for c in inner): is redundant. The int(inner, 8) call within the try block will raise a ValueError if inner contains non-octal digits, which is already handled by the except block. You can simplify the code by removing the explicit if check and relying on the try...except to handle invalid digits.

Suggested change

if not all(c in '01234567' for c in inner):

raise PrologThrow(PrologError.syntax_error("invalid octal escape", "escape_sequence/1"))

try:

value = int(inner, 8)

if value > 255:

raise PrologThrow(PrologError.syntax_error("octal escape overflow", "escape_sequence/1"))

return chr(value)

except ValueError:

raise PrologThrow(PrologError.syntax_error("invalid octal escape", "escape_sequence/1"))

try:

value = int(inner, 8)

if value > 255:

raise PrologThrow(PrologError.syntax_error("octal escape overflow", "escape_sequence/1"))

return chr(value)

except ValueError:

raise PrologThrow(PrologError.syntax_error("invalid octal escape", "escape_sequence/1"))

gemini-code-assist · 2025-12-05T11:30:11Z


-        # First, temporarily replace \\ with a placeholder
+        # First, handle octal escapes
+        import re


This import re is redundant as re is already imported at the top of the file (line 3). According to PEP 8, imports should be at the top of the file. Please remove this line.

References

PEP 8 recommends that imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants. ^(link)

gemini-code-assist · 2025-12-05T11:30:11Z

+        with pytest.raises(Exception):  # Should raise PrologThrow with syntax_error
+            parser.parse(r"test('\8\').")


Catching the generic Exception is too broad and can mask other unexpected errors. The test should be more specific by catching PrologThrow, which is the exception type raised by the parser for syntax errors. You will need to add from vibeprolog.exceptions import PrologThrow to the imports at the top of the file.

Suggested change

with pytest.raises(Exception): # Should raise PrologThrow with syntax_error

parser.parse(r"test('\8\').")

with pytest.raises(PrologThrow): # Should raise PrologThrow with syntax_error

parser.parse(r"test('\8\').")

gemini-code-assist · 2025-12-05T11:30:11Z

+        with pytest.raises(Exception):  # Should raise PrologThrow with syntax_error
+            parser.parse(r"test('\400\').")


Similar to the other test, catching Exception is too broad. Please catch the more specific PrologThrow to make the test more robust. You will need to add from vibeprolog.exceptions import PrologThrow to the imports at the top of the file.

Suggested change

with pytest.raises(Exception): # Should raise PrologThrow with syntax_error

parser.parse(r"test('\400\').")

with pytest.raises(PrologThrow): # Should raise PrologThrow with syntax_error

parser.parse(r"test('\400\').")

…rovements - Tests - Change tests to expect PrologThrow for syntax errors instead of a generic Exception. - Add explicit import from vibeprolog.exceptions import PrologThrow at test file tops. - Update invalid/octal cases to use pytest.raises(PrologThrow). - Retain tests asserting correct behavior for valid octal escapes and edge cases. - Parser - Correct single-quoted atom STRING regex to /'([^'\\]|\\[^']*|'')*'/ so that it handles embedded quotes properly. - Implement _parse_octal_escape(match) to parse octal escapes, raising PrologThrow on invalid digits or overflow (>255). - Simplify octal parsing by removing the redundant explicit digit-check and relying on ValueError for invalid digits. - Integrate _parse_octal_escape into the string unescaping flow to properly handle octal escapes. - Remove the redundant local import re line (re is already imported at the top); ensure imports remain at the top of the file. - Ensure PrologThrow is imported from vibeprolog.exceptions where needed.

kilo-code-bot

Reviewing the octal escapes implementation.

kilo-code-bot

⚠️ 2 Issues Found

Severity	Issue	Location
WARNING	Test exception catching too broad in invalid digit test	`tests/test_parser_strings.py:385`
WARNING	Test exception catching too broad in overflow test	`tests/test_parser_strings.py:398`

Recommendation: Address warning issues for better test specificity

Review Details (2 files)

Files: parser.py, tests/test_parser_strings.py (2 issues)

Previous critical issue (regex typo) resolved.
Previous medium issues (redundant check, import) resolved.
Remaining: Test exception handling should catch PrologThrow specifically instead of generic Exception.

Keep helpful comment from HEAD about dots being valid atoms in expression context, while maintaining all functional code from both branches. Amp-Thread-ID: https://ampcode.com/threads/T-753fe86b-c448-47a9-914d-c40db2805ebb Co-authored-by: Amp <amp@ampcode.com>

…ntax-tokenizer-incorrectly-handles-after-numbers feat(parser): allow dot as atom in expressions; add range operator .. and ellipsis ... support; update tokenizer, tests, and docs

Parse statements individually to prevent parser hangs

…e tests to expect PrologThrow - vibeprolog/parser.py - Change STRING regex to: STRING: /"([^"\\]|\\[^"]*)*"/ | /'([^'\\]|\\[^']*|'')*'/ - Add _parse_octal_escape(self, match) to parse octal escapes and raise PrologThrow on syntax_error or overflow - Use _parse_octal_escape within _unescape_string; remove redundant explicit octal-digit check - Remove redundant local import re; ensure re is imported at the top - Process octal escapes before other escapes - tests/test_parser_strings.py - Add: from vibeprolog.exceptions import PrologThrow - Update tests to catch PrologThrow instead of Exception for octal-escape errors: - test_octal_escape_invalid_digit - test_octal_escape_overflow

kilo-code-bot

✅ No New Issues

Changes since last review look good.

CONFORMITY_TESTING.md

…; update parsing, tests, and docs - Implement octal escape parsing in PrologTransformer: - Add _parse_octal_escape to validate and convert octal sequences like \101\ to the corresponding character. - Raise syntax errors on invalid digits or overflow (>255). - Update string unescaping: - Process octal escapes before standard escapes in _unescape_string. - Use a placeholder for handling backslashes to preserve intent. - Adjust grammar/parsing cache: - Change grammar key construction to cache by the built grammar object (grammar = self._build_grammar(operators); key = grammar) instead of using operators tuple. - Extend string regex to support octal escapes: - Update STRING rules to allow octal escapes inside both single- and double-quoted literals. - Tests added for octal escapes: - New TestOctalEscapes with cases for single/double quotes, 1- to 3-digit octals, embedded octals, max value 377 (255), invalid digits, missing terminator, overflow, and empty escapes. - Documentation updates: - docs/FEATURES.md: note octal escapes in quoted strings. - docs/GRAMMAR_COMPARISON.md: reflect compatibility and behavior of octal escapes. - docs/SYNTAX_NOTES.md: document octal escapes in Vibe-Prolog syntax notes. - Overall effect: - Enables proper parsing and representation of octal escapes within quoted literals, aligns tests and docs with new capability, and fixes grammar caching behavior to avoid inconsistencies.

…rovements - Tests - Change tests to expect PrologThrow for syntax errors instead of a generic Exception. - Add explicit import from vibeprolog.exceptions import PrologThrow at test file tops. - Update invalid/octal cases to use pytest.raises(PrologThrow). - Retain tests asserting correct behavior for valid octal escapes and edge cases. - Parser - Correct single-quoted atom STRING regex to /'([^'\\]|\\[^']*|'')*'/ so that it handles embedded quotes properly. - Implement _parse_octal_escape(match) to parse octal escapes, raising PrologThrow on invalid digits or overflow (>255). - Simplify octal parsing by removing the redundant explicit digit-check and relying on ValueError for invalid digits. - Integrate _parse_octal_escape into the string unescaping flow to properly handle octal escapes. - Remove the redundant local import re line (re is already imported at the top); ensure imports remain at the top of the file. - Ensure PrologThrow is imported from vibeprolog.exceptions where needed.

…e tests to expect PrologThrow - vibeprolog/parser.py - Change STRING regex to: STRING: /"([^"\\]|\\[^"]*)*"/ | /'([^'\\]|\\[^']*|'')*'/ - Add _parse_octal_escape(self, match) to parse octal escapes and raise PrologThrow on syntax_error or overflow - Use _parse_octal_escape within _unescape_string; remove redundant explicit octal-digit check - Remove redundant local import re; ensure re is imported at the top - Process octal escapes before other escapes - tests/test_parser_strings.py - Add: from vibeprolog.exceptions import PrologThrow - Update tests to catch PrologThrow instead of Exception for octal-escape errors: - test_octal_escape_invalid_digit - test_octal_escape_overflow

…of github.com:nlothian/Vibe-Prolog into 306-support-octal-escape-sequences-in-quoted-literals

kilo-code-bot

✅ No New Issues

Changes since last review look good.

nlothian · 2025-12-11T02:40:42Z

Fixed in #335

nlothian and others added 12 commits December 5, 2025 17:10

Apply suggestions from code review

67c3002

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

test library loading

25caf7a

Merge branch '311-parser-fails-on-19-range-syntax-tokenizer-incorrect…

1ab461a

…ly-handles-after-numbers' of github.com:nlothian/Vibe-Prolog into 311-parser-fails-on-19-range-syntax-tokenizer-incorrectly-handles-after-numbers

fix regex

30de193

fix regex

7835574

Parse statements individually to prevent parser hangs

b726e25

Merge branch '311-parser-fails-on-19-range-syntax-tokenizer-incorrect…

18dd6c3

…ly-handles-after-numbers' of github.com:nlothian/Vibe-Prolog into 311-parser-fails-on-19-range-syntax-tokenizer-incorrectly-handles-after-numbers

Handle character code literals during tokenization

5b4ff0f

Specify start rule in parse_term

9d88275

Fix dynamic operator token priority for custom operators

0c68d74

nlothian linked an issue Dec 5, 2025 that may be closed by this pull request

Support octal escape sequences in quoted literals #306

Closed

nlothian added nac Nick's Autocoder used: https://github.com/nlothian/autocoder kilocode labels Dec 5, 2025

kilo-code-bot Bot reviewed Dec 5, 2025

View reviewed changes

gemini-code-assist Bot reviewed Dec 5, 2025

View reviewed changes

kilo-code-bot Bot reviewed Dec 5, 2025

View reviewed changes

nlothian and others added 7 commits December 6, 2025 17:26

Handle Edinburgh radix tokens and reset module predicate tracking

3e63d62

Fix indentation error in atom() method

d32ee11

Merge pull request #312 from nlothian/311-parser-fails-on-19-range-sy…

4fd728c

…ntax-tokenizer-incorrectly-handles-after-numbers feat(parser): allow dot as atom in expressions; add range operator .. and ellipsis ... support; update tokenizer, tests, and docs

Merge pull request #322 from nlothian/codex/fix-parser-hang-on-lists.pl

0cb87c1

Parse statements individually to prevent parser hangs

CONFORMITY_TESTING.md

ad87df8

kilo-code-bot Bot reviewed Dec 6, 2025

View reviewed changes

nlothian and others added 2 commits December 6, 2025 18:04

Merge pull request #324 from nlothian/fix/compliance_report

818b208

CONFORMITY_TESTING.md

tests pass

6015bc6

nlothian added 5 commits December 7, 2025 05:37

tests pass

f623009

Merge branch '306-support-octal-escape-sequences-in-quoted-literals' …

3b55643

…of github.com:nlothian/Vibe-Prolog into 306-support-octal-escape-sequences-in-quoted-literals

kilo-code-bot Bot reviewed Dec 7, 2025

View reviewed changes

nlothian closed this Dec 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(octal escapes): add support for octal escapes in quoted literals; update parsing, tests, and docs#323

feat(octal escapes): add support for octal escapes in quoted literals; update parsing, tests, and docs#323
nlothian wants to merge 27 commits into
mainfrom
306-support-octal-escape-sequences-in-quoted-literals

nlothian commented Dec 5, 2025

Uh oh!

kilo-code-bot Bot left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Uh oh!

kilo-code-bot Bot left a comment

Uh oh!

kilo-code-bot Bot left a comment

Uh oh!

kilo-code-bot Bot left a comment

Uh oh!

kilo-code-bot Bot left a comment

Uh oh!

nlothian commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	STRING: /"([^"\\]\|\\[^"])"/ \| /'([^'\\]\|\\[^']\|'' )'/
	STRING: /"([^"\\]\|\\[^"])"/ \| /'([^'\\]\|\\[^']\|'')'/

		with pytest.raises(Exception): # Should raise PrologThrow with syntax_error
		parser.parse(r"test('\8\').")

		with pytest.raises(Exception): # Should raise PrologThrow with syntax_error
		parser.parse(r"test('\400\').")

Conversation

nlothian commented Dec 5, 2025

Uh oh!

kilo-code-bot Bot left a comment

Choose a reason for hiding this comment

✅ No Issues Found

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot left a comment

Choose a reason for hiding this comment

⚠️ 2 Issues Found

Uh oh!

kilo-code-bot Bot left a comment

Choose a reason for hiding this comment

✅ No New Issues

Uh oh!

kilo-code-bot Bot left a comment

Choose a reason for hiding this comment

✅ No New Issues

Uh oh!

nlothian commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant