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

Do not merge: Impl inst macro #56

Closed
wants to merge 5 commits into from
Closed

Conversation

terasakisatoshi
Copy link
Member

No description provided.

Create a string from `expr`. It tries to output something similar to we type in Julia REPL.
If `expr` contains `@comment <message>`, it is transformed into `# <message>`.
"""
macro inst(expr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel the wording inst is a little ambiguous, and 英辞郎 does not list "instruction" as a full form of "inst"..

image
https://eowf.alc.co.jp/search?q=inst.

Is it bothersome to rename it like instruction?

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about deparse? That would be a little longer than inst, but I think it describes its behavior correctly.

Copy link
Member Author

@terasakisatoshi terasakisatoshi Apr 10, 2023

Choose a reason for hiding this comment

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

Is it bothersome to rename it like instruction?

How about providing both @instruction and @i? The latter macro @i is alias to @instruction, which can be implemented:

module Replay
export @instruction

...
"""
alias to @instruction
"""
const var"@i" = var"@instruction"
...
end

Usage:

julia> using Replay

julia> using Replay: @i

julia> @instruction println("Hi")
"println(\"Hi\")"

julia> @i println("Hi")
"println(\"Hi\")"

Copy link
Member Author

Choose a reason for hiding this comment

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

If I could, I would like to export @i.

Copy link
Collaborator

@hyrodium hyrodium Apr 10, 2023

Choose a reason for hiding this comment

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

How about providing both @instruction and @i?

If I could, I would like to export @i.

I think exporting short variables such as @i is so anti-pattern that other package does not exports and I feel okay:joy:

Do you have any thoughts on deparse?
If this is still long to type, how about defining const var"@d" = var"@deparse"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think exporting short variables such as @i is so anti-pattern that other package does not exports and I feel okay😂

(´・ω・`) I'm lazy.

Do you have any thoughts on deparse?
If this is still long to type, how about defining const var"@d" = var"@deparse"?

I can't decide right now.
🤔

Let me think about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have discussed this with GPT-4. Here's a screenshot of that.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

After googling and pondering for a moment, I found it makes sense to use "deparse". R lang provides a "deparse" function that converts an expression into a character string. I could learn about what "deparse" was. Thank you for your suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

However, I need to re-create another pull request because of the name of the branch contains "inst". No worry, It's a piece of cake. I will do that.

test/inst.jl Outdated Show resolved Hide resolved
terasakisatoshi and others added 2 commits April 10, 2023 23:25
Co-authored-by: Yuto Horikawa <[email protected]>
@terasakisatoshi terasakisatoshi mentioned this pull request Apr 13, 2023
@terasakisatoshi terasakisatoshi changed the title Impl inst macro Do not merge: Impl inst macro Apr 22, 2023
@hyrodium hyrodium mentioned this pull request Oct 16, 2023
@terasakisatoshi terasakisatoshi added the invalid This doesn't seem right label Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants