Skip to content

Address PR #238 review-thread feedback in drawing example and interactive input helper#337

Closed
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-review-comments
Closed

Address PR #238 review-thread feedback in drawing example and interactive input helper#337
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-review-comments

Conversation

Copilot AI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

This PR applies all requested updates from the linked review thread on PR #238, with no unrelated scope expansion. It tightens drawing schema validity, aligns discriminant conventions, and removes two renderer/interactive edge-case failure points.

  • Schema tightening + consistency

    • Restricts Style.line_style to Literal["solid", "dashed", "dotted"] instead of free-form str.
    • Aligns unknown-case discriminant with existing examples by changing UnknownText.type to Literal["Unknown"].
    • Updates Arrow docstring to match implemented behavior (arrow head at end).
  • Renderer hardening

    • Replaces dash-pattern direct indexing with a safe fallback to solid line when line_style is unexpected.
  • Lint-safe optional readline import

    • Marks optional readline import as intentionally unused to avoid F401 noise while preserving side-effect behavior.
# Before
dash=dash_pattern[getattr(arrow.style, "line_style", None) or "solid"]

# After
dash=dash_pattern.get(getattr(arrow.style, "line_style", None) or "solid", "")

Copilot AI changed the title [WIP] Fix code based on review comments Address PR #238 review-thread feedback in drawing example and interactive input helper Jun 1, 2026
Copilot AI requested a review from robgruen June 1, 2026 21:55
@robgruen

robgruen commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

I made these changes in @gvanrossum 's fork and so they're now in #238

@robgruen robgruen closed this Jun 1, 2026
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