Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define STRFTIME/MOMENT/MOMENT-MS/RSS-DATE #163

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

shawwn
Copy link
Member

@shawwn shawwn commented Feb 16, 2019

(Also define str and leftpad)

@shawwn
Copy link
Member Author

shawwn commented Feb 16, 2019

I wanted to get another set of eyes on this before merging. strftime isn't meant to be a complete implementation, but it's pretty close.

@akkartik
Copy link
Member

My eyes are not very useful when there aren't tests to run. But these are new functions so no worries. I assume you're running them in laarc? Please remember to backport bugfixes as you run into them.

Oh also, it may be convenient to add some examples just from things you try out at the repl. Easier than installing hg and unit-test.arc.

@@ -648,6 +648,10 @@ More convenient form of [[coerce]] with arguments reversed; doesn't need
"Converts 'x' into a symbol."
(coerce x 'sym))

(def str (x)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this, when we have string?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly. (coerce x 'string) behaves differently from (+ "" x).

Copy link
Member

Choose a reason for hiding this comment

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

In what situation?

Copy link
Member Author

Choose a reason for hiding this comment

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

In no situations. Never mind, I was mistaken. :)

One benefit of adding a str function is that it can be a place to do things like printing a hash table using curly brace syntax. But that's work for some other time. I'll get rid of str.

Copy link
Member

Choose a reason for hiding this comment

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

Also, feel free to overload string for those purposes.

(I don't particularly what the function is called. But it seems good to have one strong default.)

@shawwn
Copy link
Member Author

shawwn commented Feb 16, 2019

I assume you're running them in laarc?

Yup! Previously I was shelling out to date using system, but that's very slow.

Please remember to backport bugfixes as you run into them.

No need to port bug fixes when the code is flawless. /s

(Good point! I'll be sure to port any fixes that come up.)

Oh also, it may be convenient to add some examples just from things you try out at the repl. Easier than installing hg and unit-test.arc.

Good idea!

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