Skip to content

Preserver previous command results (avoid rerunning commands)#128

Open
owen2345 wants to merge 9 commits into
crystal-community:masterfrom
owen2345:master
Open

Preserver previous command results (avoid rerunning commands)#128
owen2345 wants to merge 9 commits into
crystal-community:masterfrom
owen2345:master

Conversation

@owen2345

@owen2345 owen2345 commented Apr 13, 2021

Copy link
Copy Markdown

Features

  • feat: support to print previous command result with __ or __previous_result
  • feat: Preserve regular command results as serialized values and make them available to be used by next commands (avoid re running commands)
  • feat: Scan declared variables inside a command to make available in next commands

TODO:

  • Add tests
  • Calculate all variables declared inside the command (currently only takes the first variable using regex)

Samples:

  $> 10 # => 10
  $> puts "previous result: #{__}" # => previous result: 10

  $> var1 = "Var1" # => ""
  $> puts "var1 is: #{var1}" # => var1 is: Var1

  # Array support
  $> arr = [1,2,3]
  $> puts "arr result: #{arr} and size: #{arr.size}" # => arr result: [1, 2, 3] and size: 3

  # Hash support
  $> hash = { s: "String", number: 10 }
  $> puts "result: #{hash} and number: #{hash["number"]}" # => result: {s: "String", number: 10} and number: 10

  #complex objects (Amber app)
  $> require "./config/application.cr"
  $> qty_articles = Article.count
  $> article = Article.create(title: "Article 1")
  $> puts "qty_articles is #{qty_articles} vs #{Article.count}" # qty_articles is 0 vs 1
  $> puts "last article is: #{article.title}" # last article is: Article 1

Status: In progress

@jwoertink

Copy link
Copy Markdown
Collaborator

Well this is interesting! I'll come back to review a bit later, but my first question is what is the difference between _p and __ (two underscores we currently use)?

@owen2345

Copy link
Copy Markdown
Author

Well difference between _p and __ is that __ can not be used as a variable in the next command, against _p which can be used as a normal variable like:

puts "previous value: #{_p}" 

Btw: I tried to replace _p into __ but unfortunately crystal does not accept as a valid variable name, either _;

@jwoertink

Copy link
Copy Markdown
Collaborator

__ can not be used as a variable in the next command

I'm not sure that I follow exactly. I know this works in Crystal

__ = 1

__ += 1

puts __

Here's the PR where it was added in originally #63

I just want to make sure that if we're making this change, it's not a 1 to 1 of what we have now for the sake of "a better name" or something along those lines. I'd rather not have 2 ways to do this, so if we're going to add in _p, then we should remove __ with an explanation as to why _p is a better solution. Hope that makes sense.

@owen2345

Copy link
Copy Markdown
Author

@jwoertink you were right! Sorry my mistake. I was too tired yesterday then I could not test it very well.

@jwoertink

Copy link
Copy Markdown
Collaborator

No worries! I just want to make sure we're getting this right. Thanks for contributing ❤️

@jwoertink jwoertink 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.

Sorry it took me some time to come back and review this. It seems like a pretty decent start. I was able to create a time object and it persisted between calls which is an amazing improvement. It looks like there's still some issues though.

For example:

icr(1.0.0) > t = Time.utc

 => 2021
icr(1.0.0) > t

 => 2021
icr(1.0.0) > t.to_s("%F")

 => "2021-06-14"

It seems the default output of a Time object is just the year which could be a bit confusing.

Trying to use both __ and __previous_result just return this error:

icr(1.0.0) > __
Showing last frame. Use --error-trace for full trace.

In .icr_Vbx6IjLdROuGGOgDEDzfUA.cr:14:3

 14 | __previous_result = (__previous_result);
      ^
Error: expression has no effect
icr(1.0.0) > __previous_result
Showing last frame. Use --error-trace for full trace.

In .icr_Vbx6IjLdROuGGOgDEDzfUA.cr:14:3

 14 | __previous_result = (__previous_result);
      ^
Error: expression has no effect

Lastly, trying to define a simple class in two lines seems to bork the whole session:

icr(1.0.0) > class Test
icr(1.0.0) >   end
There was a problem expanding macro 'macro_4538303136'

Code in macro 'init'

 1 | {% if T < Enum %}
     ^
Called macro defined in macro 'init'

 1 | {% if T < Enum %}

Which expanded to:

 > 422 |       when .== Class then new Class
 > 423 |
 > 424 |       when .== GC:Module then new GC:Module
                          ^
Error: unexpected token: Module (expecting ',', ';' or '

')

After doing that, no other command works until I quit and restart.

Overall I think this will be an amazing update, but I think it'll need just a bit more fine tuning before we merge it in.

Ping me if you have any questions, or need anymore testing done. Thanks for your work on this!

@jwoertink jwoertink mentioned this pull request Sep 25, 2021
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.

2 participants