FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

builtin: Implement builtin_ascii by corona10 · Pull Request #66 · go-python/gpython · GitHub

builtin: Implement builtin_ascii - #66

Merged
corona10 merged 1 commit into
go-python:masterfrom
corona10:ascii
Aug 4, 2019
Merged

builtin: Implement builtin_ascii#66
corona10 merged 1 commit into
go-python:masterfrom
corona10:ascii

Conversation

corona10 commented Jul 26, 2019
edited
Loading

Copy link
Copy Markdown
Collaborator

Implement builtin_ascii

corona10 added the WIP label Jul 26, 2019
corona10 changed the title [WIP] builtin: Implement builtin_ascii builtin: Implement builtin_ascii Jul 26, 2019
corona10 removed the WIP label Jul 26, 2019
corona10 requested a review from ncw July 26, 2019 15:07

Copy link
Copy Markdown
Collaborator Author

@ncw
Please take a look!

corona10 requested a review from a team July 26, 2019 15:09

codecov-io commented Jul 26, 2019
edited by codecov Bot
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.63%. Comparing base (c5b8c68) to head (a3dfc79).

Files with missing lines Patch % Lines
builtin/builtin.go 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
+ Coverage   68.45%   68.63%   +0.18%     
==========================================
  Files          59       59              
  Lines       10495    10512      +17     
==========================================
+ Hits         7184     7215      +31     
+ Misses       2804     2789      -15     
- Partials      507      508       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ncw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

See inline for adapting an existing function to do this

Comment thread builtin/tests/builtin.py
Comment thread builtin/builtin.go Outdated
Comment thread builtin/builtin.go

Copy link
Copy Markdown
Collaborator Author

@ncw
Thanks for the kind review.
I 've added additional tests.
But I didn't reflect some of your reviews that I 've not understood yet.
I left comments on that!
Please take a look.

Copy link
Copy Markdown
Collaborator Author

I've found this code does not work well.

>>> ascii('\U+10001')
"'\\U00010001'"

corona10 changed the title builtin: Implement builtin_ascii [WIP] builtin: Implement builtin_ascii Jul 28, 2019
corona10 changed the title [WIP] builtin: Implement builtin_ascii builtin: Implement builtin_ascii Jul 28, 2019
corona10 changed the title builtin: Implement builtin_ascii [WIP] builtin: Implement builtin_ascii Jul 28, 2019
corona10 added the WIP label Jul 28, 2019
corona10 changed the title [WIP] builtin: Implement builtin_ascii builtin: Implement builtin_ascii Jul 31, 2019

Copy link
Copy Markdown
Collaborator Author

@ncw
I've added StringEscape which can be used for repr or ascii.
Note that StringEscape which need to ascii is below code.

If there is something I missed. Please let me know!

func stringEscape(a py.String) string {
    s := string(a)
    var out bytes.Buffer
    for _, c := range s {
        switch {
        case c < 0x20:
            switch c {
            case '\t':
                out.WriteString(`\t`)
            case '\n':
                out.WriteString(`\n`)
            case '\r':
                out.WriteString(`\r`)
            default:
                out.WriteRune(c)
            }
        case c < 0x100:
            out.WriteRune(c)
        case c < 0x10000:
            fmt.Fprintf(&out, "\\u%04x", c)
        default:
            fmt.Fprintf(&out, "\\U%08x", c)
        }
    }
    return out.String()
}

corona10 removed the WIP label Jul 31, 2019

ncw commented Aug 4, 2019

Copy link
Copy Markdown
Collaborator

LGTM - thank you - please merge :-)

corona10 merged commit fff175c into go-python:master Aug 4, 2019
corona10 deleted the ascii branch August 4, 2019 11:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL