ubiquity / ubiquibot

Putting the 'A' in 'DAO'
https://github.com/marketplace/ubiquibot
MIT License
16 stars 59 forks source link

Strings #862

Open Keyrxng opened 9 months ago

Keyrxng commented 9 months ago

Resolves #837

Quality Assurance: test case written

Just came to me while sorting types for another piece of work

netlify[bot] commented 9 months ago

Deploy Preview for ubiquibot-staging ready!

Name Link
Latest commit 0a83bf7229a421919acac68c04ff30379ed67a81
Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/65426105a9b711000814be38
Deploy Preview https://deploy-preview-862--ubiquibot-staging.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Keyrxng commented 9 months ago

idk why I've got 3 week old commits in this, my dev branch is up to date with upstream and only made this branch this afternoon, fixed now won't happen again.

seems that CI is failing because of '.test.ts' but is needed for jest, I can remove the test or I'm open to ideas

0x4007 commented 9 months ago

This is interesting but I've never heard of f strings. Perhaps you can share some of your favorite resources on it and explain why you were inspired to take this approach?

It does look awfully pythonic which I have mixed feelings on. Perhaps there is a TypeScript-esqe parallel that we can draw inspiration from?

Keyrxng commented 9 months ago

The inspiration comes from using it with gpt prompts when building with langchain, it's readable and robust. The changes from Lang to this was just streamlining what we needed and optimizing things a little.

I don't think there is a TS parallel to draw from no at least not to my knowledge, it's a 'formatted string literal' similar to JS/TS \`${}`\ but from what I read we cannot use that approach and need to use something similar to Python's % string formatting.

I tried to find what you meant when you said this but I couldn't so I don't understand the issue with just using ${} so I figured this was a more elegant way to do things and it can be repurposed as across the bot as opposed to just the E2E tests

I presume the issue with using ${} is due to html formatting, if this is the reason then .replace() is the only built-in option and then we'd just replace HTML entities? I don't like the sound of it personally but if it's the preference then this could be refactored but I think this is superior

Keyrxng commented 8 months ago

It just came to me that there is ofc a way to handle this with just TS and this is probably the sleekest also no longer caring about trying to escape although if using it for prompt template formatting then we should just use different delimiters <> as opposed to {} when prompting AI