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

Edit command doesn't write to history #1749

Closed
jackkinsella opened this issue Mar 14, 2018 · 13 comments
Closed

Edit command doesn't write to history #1749

jackkinsella opened this issue Mar 14, 2018 · 13 comments

Comments

@jackkinsella
Copy link
Contributor

I spend most of my Ruby time programming from pry and I recently discovered that the edit command lets me craft a piece of code in good old Vim before executing it.

A feature that I'm missing (or perhaps that I just don't know how to enable) is the ability to press "up" then "enter" to quickly re-execute the last line of code. For me, that's a staple of REPL programming - be that in shell, postgres, or pry.

I know that this feature doesn't make sense for advanced uses of the edit command, like editing a file: All I'm talking about here is modifying the basic argument-less edit command to write its contents to history.

If this is something you're interested in, I'd be happy to send a PR.

@ghost
Copy link

ghost commented Mar 14, 2018

Cool idea! But by just pushing with Pry.history.push it would add newlines in a single history entry (which I do not find bad in whatever). You could also split the code line by line and then push each line the irb style. This should be discussed.

@jackkinsella
Copy link
Contributor Author

jackkinsella commented Mar 14, 2018

Awesome! My two cents is that the most intuitive approach would be one where the whole command can be repeated with two keystrokes – that's the workflow that I've come to expect from other REPLs. This is gonna entail pushing newlines into the history entry, as you said.

I guess we could give it a shot and see how people respond - after all, we could always add a configuration option to modify this behavior in future.

@jackkinsella
Copy link
Contributor Author

To expand on what I said above – I use keyboard shortcuts in TMUX to press up and enter in other screens, and I'm certainly not the only programmer who takes this approach. The way of dealing with new-lines that keeps this workflow working with pry would be my preferred solution.

@ghost
Copy link

ghost commented Mar 14, 2018

Then I guess there should be a way to edit a multiline command saved in history. And I'm not sure that if someone were to press up and down the cursor would behave as expected without minor tweaks.

Note that I'm not related to Pry. I'm just discussing that specific issue.

@banister
Copy link
Member

I've been thinking about this too -- if you guys want to implement this feature i'd be happy to review and possibly merge it too :)

@jackkinsella
Copy link
Contributor Author

Kewl! I'll familiarize myself with Pry's codebase when I get a free afternoon and then dig in :)

@ghost
Copy link

ghost commented Mar 15, 2018

Here is what I get so far: https://asciinema.org/a/zll7fXAGp9IrEd0khlF7ZRAql. Just by adding Pry.history.push content in repl_edit. As you can see small tweaks can be achieved. But I think there should be a command to edit a given history entry. Like edit last or edit 23. Editing "the last entry that has been edited in an external editor" can apparently be achieved using edit -r. I'm still trying to find a way to edit history entries.

asciicast

@ghost
Copy link

ghost commented Mar 15, 2018

So I just got back from my english class and I managed to get the whole thing to work. With edit -s 12, we can edit the given entry. I created a repl_edit method that call on repl_edit_ because I couldn't figure how to introduce a new argument (which is the text to edit) without breaking the other calls to repl_edit. Is that the right thing to do ? Also if the number of argument is invalid, should I throw a CommandError ?

Note that positive indexes are only valid when you read them using hist -a. (I decided to not accept positive numbers due to possible confusion when using hist with different parameters.) For the negative ones, -1 is the last expression and so on...

Here is a video: https://asciinema.org/a/rrRTqwBj9ItnAKv6tEIprTLuL

asciicast

@ghost
Copy link

ghost commented Mar 15, 2018

Finally I added an argument to repl_edit, finding there was no calls I could break.

@ghost
Copy link

ghost commented Mar 22, 2018

Any update ? Anyone ?

@jackkinsella
Copy link
Contributor Author

@breakfast1 - I read through your PR and it looked like you were going for a more ambitious change than what I proposed here. I pushed an MVP PR that just addresses the basics. I suggest you create a separate issue and PR re: the new flag you're proposing.

@ghost
Copy link

ghost commented Mar 25, 2018

I'll leave it like that as I have no time to re issue and fix the code. But I agree with you. Thanks for replying.

I mentioned your PR in mine and closed it.

banister added a commit that referenced this issue May 18, 2018
@kyrylo
Copy link
Member

kyrylo commented Nov 3, 2018

Fixed by #1752.

@kyrylo kyrylo closed this as completed Nov 3, 2018
kyrylo added a commit that referenced this issue Nov 3, 2018
kyrylo added a commit that referenced this issue Nov 3, 2018
kyrylo added a commit that referenced this issue Nov 4, 2018
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 17, 2018
pkgsr change:
* Remove @Prefix@ from ALTERNATIVES file.

### [v0.12.2][v0.12.2] (November 12, 2018)

#### Bug fixes

* Restore removed deprecations, which were removed by accident due to a bad
  rebase.

### [v0.12.1][v0.12.1] (November 12, 2018)

#### Bug fixes

* Stopped creating a new hash each time `Pry::Prompt#[]` is invoked
  ([#1855](pry/pry#1855))
* Fixed `less` pager not working when it's available
  ([#1861](pry/pry#1861))

### [v0.12.0][v0.12.0] (November 5, 2018)

#### Major changes

* Dropped support for Rubinius ([#1785](pry/pry#1785))

#### Features

* Added a new command, `clear-screen`, that clears the content of the screen Pry
  is running in regardless of platform (Windows or UNIX-like)
  ([#1723](pry/pry#1723))
* Added a new command, `gem-stat`, that prints gem statistics such as gem
  dependencies and downloads ([#1707](pry/pry#1707))
* Added support for nested exceptions for the `wtf` command
  ([#1791](pry/pry#1791))
* Added support for dynamic prompt names
  ([#1833](pry/pry#1833))

  ```rb
  # pryrc
  Pry.config.prompt_name = Pry.lazy { rand(100) }

  # Session
  [1] 80(main)>
  [2] 87(main)>
  [3] 30(main)>
  ```
* Added support for XDG Base Directory Specification
  ([#1609](pry/pry#1609),
  [#1844](pry/pry#1844),
  ([#1848](pry/pry#1848)))
* Removed the `simple-prompt`. Use `change-prompt simple` instead. The
  `list-prompt` command was removed and embedded as `change-prompt --list`
  ([#1849](pry/pry#1849))

#### API changes

* The following methods started accepting the new optional `config` parameter
  ([#1809](pry/pry#1809)):
  * `Pry::Helpers.tablify(things, line_length, config = Pry.config)`
  * `Pry::Helpers.tablify_or_one_line(heading, things, config = Pry.config)`
  * `Pry::Helpers.tablify_to_screen_width(things, options, config = Pry.config)`
  * `Pry::Helpers::Table.new(items, args, config = Pry.config)`

  You are expected to pass a session-local `_pry_.config` instead of the global
  one.

* Added new method `Pry::Config.assign`, for creating a Config non-recursively
  ([#1725](pry/pry#1725))
* Added `Pry.lazy`, which is a helper method for values that need to be
  calculated dynamically. Currently, only `config.prompt_name` supports it
  ([#1833](pry/pry#1833))
* `Pry::Prompt` responds to `.[]`, `.all` & `.add` now. The `Pry::Prompt.add`
  method must be used for implementing custom prompts. See the API in the
  documentation for the class ([#1846](pry/pry#1846))

#### Breaking changes

* Deleted the `Pry::Helpers::Text.bright_default` alias for
  `Pry::Helpers::Text.bold` ([#1795](pry/pry#1795))
* `Pry::Helpers.tablify_to_screen_width(things, options, config = Pry.config)`
  requires `options` or `nil` in place of them.
* `Pry::Helpers::Table.new(items, args, config = Pry.config)` requires `args`
  or `nil` in place of them.
* Completely revamped `Pry::HistoryArray`
  ([#1818](pry/pry#1818)).
  * It's been renamed to `Pry::Ring`
    ([#1817](pry/pry#1817))
  * The implementation has changed and as result, the following methods were
    removed:
    * `Pry::Ring#length` (use `Pry::Ring#count` instead)
    * `#empty?`, `#each`, `#inspect`, `#pop!`, `#to_h`
  * To access old Enumerable methods convert the ring to Array with `#to_a`
  * Fixed indexing for elements (e.g. `_pry_.input_ring[0]` always return some
    element and not `nil`)
* Renamed `Pry.config.prompt_safe_objects` to `Pry.config.prompt_safe_contexts`
* Removed deprecated `Pry::CommandSet#before_command` &
  `Pry::CommandSet#after_command` ([#1838](pry/pry#1838))

#### Deprecations

* Deprecated `_pry_.input_array` & `_pry_.output_array` in favour of
  `_pry_.input_ring` & `_pry_.output_ring` respectively
  ([#1814](pry/pry#1814))
* Deprecated `Pry::Command#text`. Please use `#black`, `#white`, etc. directly
  instead (as you would with helper functions from `BaseHelpers` and
  `CommandHelpers`) ([#1701](pry/pry#1701))
* Deprecated `_pry_.input_array` & `_pry_.output_array` in favour of
  `_pry_.input_ring` and `_pry_.output_ring` respectively
  ([#1817](pry/pry#1817))
* Deprecated `Pry::Platform`. Use `Pry::Helpers::Platform` instead. Note that
  `Pry::Helpers::BaseHelpers` still includes the `Platform` methods but emits a
  warning. You must switch to `Pry::Helpers::Platform` in your code
  ([#1838](pry/pry#1838),
  ([#1845](pry/pry#1845)))
* Deprecated `Pry::Prompt::MAP`. You should use `Pry::Prompt.all` instead to
  access the same map ([#1846](pry/pry#1846))

#### Bug fixes

* Fixed a bug where `cd Hash.new` reported `self` as an instance of Pry::Config
  in the prompt ([#1725](pry/pry#1725))
* Silenced the `Could not find files for the given pattern(s)` error message
  coming from `where` on Windows, when `less` or another pager is not installed
  ([#1767](pry/pry#1767))
* Fixed possible double loading of Pry plugins' `cli.rb` on Ruby (>= 2.4) due to
  [the `realpath` changes while invoking
  `require`](https://bugs.ruby-lang.org/issues/10222)
  ([#1762](pry/pry#1762),
  [#1774](pry/pry#1762))
* Fixed `NoMethodError` on code objects that have a comment but no source when
  invoking `show-source` ([#1779](pry/pry#1779))
* Fixed `negative argument (ArgumentError)` upon pasting code with tabs, which
  used to confuse automatic indentation
  ([#1771](pry/pry#1771))
* Fixed Pry not being able to load history on Ruby 2.4.4+ when it contains the
  null character ([#1789](pry/pry#1789))
* Fixed Pry raising errors on `cd`'ing into some objects that redefine
  `method_missing` and `respond_to?`
  ([#1811](pry/pry#1811))
* Fixed bug when indentation leaves parts of input after pressing enter when
  Readline is enabled with mode indicators for vi mode
  ([#1813](pry/pry#1813),
  [#1820](pry/pry#1820),
  [#1825](pry/pry#1825))
* Fixed `edit` not writing to history
  ([#1749](pry/pry#1749))

#### Other changes

* Deprecated the `Data` constant to match Ruby 2.5 in the `ls` command
  ([#1731](pry/pry#1731))
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

No branches or pull requests

3 participants