Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Trickybrain Creating the LineReader file #14

Merged
merged 25 commits into from
Feb 28, 2024
Merged

Trickybrain Creating the LineReader file #14

merged 25 commits into from
Feb 28, 2024

Conversation

Trickybrain
Copy link
Collaborator

Trickybrain Creating the LineReader file

@nedtwigg
Copy link
Member

The pyright errors here are saving you from a real problem:

/home/runner/work/selfie-python-wip/selfie-python-wip/python/selfie-lib/selfie_lib/LineReader.py
/home/runner/work/selfie-python-wip/selfie-python-wip/python/selfie-lib/selfie_lib/LineReader.py:7:30 - error: Cannot assign member "unix_newlines" for type "LineTerminatorAware*"
Type "Literal[True]" cannot be assigned to type "() -> (() -> ...)" (reportAttributeAccessIssue)
/home/runner/work/selfie-python-wip/selfie-python-wip/python/selfie-lib/selfie_lib/LineReader.py:10:34 - error: Cannot assign member "unix_newlines" for type "LineTerminatorAware*"
Type "Literal[False]" cannot be assigned to type "() -> (() -> ...)" (reportAttributeAccessIssue)
/home/runner/work/selfie-python-wip/selfie-python-wip/python/selfie-lib/selfie_lib/LineReader.py:21:38 - error: Cannot assign member "unix_newlines" for type "LineTerminatorAware*"

Look at lines 7/10/21. Let me know if you'd like another hint.

@nedtwigg
Copy link
Member

Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

Getting closer! The errors that pyright is giving you in CI are all helpful.

You don't have to push to CI to find them, you can also run them locally with poetry run pyright.

python/selfie-lib/selfie_lib/LineReader.py Outdated Show resolved Hide resolved
@Trickybrain Trickybrain marked this pull request as ready for review February 27, 2024 22:13
Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

This looks excellent! Two things it needs to be finished

  • small nit about public/private
  • it is missing the def get_line_number(self) -> int: method mentioned earlier. The later steps will be very difficult to debug without it. We can count the lines ourselves, we don't need Python's library implementation to do it.

python/selfie-lib/selfie_lib/LineReader.py Outdated Show resolved Hide resolved
@nedtwigg
Copy link
Member

nedtwigg commented Feb 28, 2024

Great work! Take a look at the small commit I pushed up above, private is two underscores not one.

  • Now that Convert LineReader #5 is done, you can start PerCharacterEscaper, the next piece of Epic: .ss file read/write #7
  • Create an issue for it
  • link to the kotlin code that you're going to port, and any tests that are relevant to it
  • then start writing python code

@nedtwigg nedtwigg merged commit 80cb448 into diffplug:main Feb 28, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants