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

Creating SourceFile#20

Merged
nedtwigg merged 19 commits intodiffplug:mainfrom
SDelgado-21:test
Mar 14, 2024
Merged

Creating SourceFile#20
nedtwigg merged 19 commits intodiffplug:mainfrom
SDelgado-21:test

Conversation

@nedtwigg
Copy link
Copy Markdown
Member

nedtwigg commented Feb 29, 2024

  • Language enum -> port, only value is python
  • EscapeLeadingWhitespace enum -> port, only value is never
  • ignore removeSelfieOnceComments
  • The thing we want is fun parseToBeLike(lineOneIndexed: Int): ToBeLiteral {
  • Key point: SourceFileToBeTest

@nedtwigg
Copy link
Copy Markdown
Member

Your choice if this PR supports multiline strings or not.

Copy link
Copy Markdown
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.

Good progress!

Comment thread python/selfie-lib/selfie_lib/EscapeLeadingWhitespace.py Outdated
Comment thread python/selfie-lib/selfie_lib/EscapeLeadingWhitespace.py Outdated
Comment thread python/selfie-lib/selfie_lib/Literals.py Outdated
Comment thread python/selfie-lib/selfie_lib/SourceFile.py Outdated
Copy link
Copy Markdown
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.

Good progress! Some nitpick comments, looks like you'll need to at least mock-out LiteralValue so that the tests can run.

Comment thread python/selfie-lib/selfie_lib/EscapeLeadingWhitespace.py Outdated
Comment thread python/selfie-lib/selfie_lib/EscapeLeadingWhitespace.py Outdated
Comment thread python/selfie-lib/selfie_lib/EscapeLeadingWhitespace.py Outdated
Comment thread python/selfie-lib/selfie_lib/Literals.py Outdated
@nedtwigg
Copy link
Copy Markdown
Member

nedtwigg commented Mar 9, 2024

This PR isn't about correctly implementing LiteralValue / LiteralFormat, but having a simple temporary implementation of them might make it easier. At the very least, it will be important that their type signatures are correct. See the PEP 695 docs.

Copy link
Copy Markdown
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.

Excellent progress! This was a hard one, and you got the hardest parts working!

Comment thread python/selfie-lib/selfie_lib/Literals.py Outdated
Comment thread python/selfie-lib/selfie_lib/SourceFile.py Outdated
Comment thread python/selfie-lib/selfie_lib/SourceFile.py Outdated
Comment thread python/selfie-lib/tests/SourceFile_test.py Outdated
Comment thread python/selfie-lib/tests/SourceFile_test.py Outdated
Copy link
Copy Markdown
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.

Excellent work! I think this is ready merge. @SDelgado-21 if you agree click "Ready for review" and then "Merge pull request".

I would set the commit message to "Ported SourceFile (#20)" but it's not crucial.

@SDelgado-21 SDelgado-21 marked this pull request as ready for review March 14, 2024 04:31
@nedtwigg nedtwigg merged commit eb4560c into diffplug:main Mar 14, 2024
@nedtwigg nedtwigg mentioned this pull request Mar 15, 2024
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