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.

Sign up for GitHub

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

Jump to bottom

{after,before}_command hooks should be unified with Pry::Hooks system #651

Closed
banister opened this issue Jul 15, 2012 · 4 comments · Fixed by #1391
Closed

{after,before}_command hooks should be unified with Pry::Hooks system #651

banister opened this issue Jul 15, 2012 · 4 comments · Fixed by #1391
Labels
refactor
Milestone
v1.0.0

Comments

@banister
Copy link
Member

Pry.commands.after_command("edit") becomes Pry.hooks.add_hook(:after_edit, :name)

Pry.commands.after_command("!!!") becomes Pry.hooks.add_hook(:"after_!!!", :name)

Note the symbol 'string' in the last example as :after_!!! is not a valid symbol

Consider making hook names 'indifferent' to string/symbols, so "after_!!!" is the same as :"after_!!!"
@Mon-Ouie
Copy link
Member

There's no reason to force yourself to use symbols for hooks — as long as the object is a valid hash-key, you're fine.

Pry.hooks.add_hook([:after, command], name)

@banister
Copy link
Member Author

@Mon-Ouie yeah, it's just that we already have a bunch of hooks defined already that are all symbols like, this: :after_eval, :before_session, :when_started etc etc

I'd like to try to keep consistent. Also there's the issue of if we use strings for some hooks and symbols for others, it opens up annoying bugs where someone uses a string "after_eval" rather than a Symbol by pure accident...it makes me think not differentiating between strings/symbols would guard well against this class of bug

@banister banister mentioned this issue Aug 7, 2012
Closed
moss added a commit to moss/pry that referenced this issue Dec 23, 2013
@moss
Use Pry::Hooks for before/after command hooks
5b65599 
This lets you use the standard Pry.hooks.add_hook in place of
Pry.commands.before_command and Pry.commands.after_command, addressing
at least part of issue pry#651

Note that this commit does not yet remove the old command hook system.
@moss
Copy link

moss commented Dec 23, 2013

Hi! I did some work on this today, but I think I've got a question for someone with more experience on the project:

Should we drop the existing Pry.commands.before_command and after_command methods, or keep them working in parallel with the new system, maybe with deprecation warnings? Making a clean break seems like the nicest approach code-wise, but I don't want to break backwards compatibility either.

@ConradIrwin
Copy link
Member

The current plan for the next release is 1.0, so we're probably happy to break backward compatibility in small ways.

That said, unless it's really hard, I'd just leave them there.

@rf- rf- modified the milestones: v0.10.0, 0.9.11, v1.0.0 Apr 29, 2014
kyrylo added a commit that referenced this issue Mar 11, 2015
@kyrylo
Refactor command hooks so they could use Pry::Hooks (newer API)
47d8103 
Fixes #651 ({after,before}_command hooks should be unified with
Pry::Hooks system)

* The support for the `Pry.commands.{after,before}_command` API was kept
* Hooks defined via the older API use random names
* The order of hooks execution was changed from LIFO to FIFO
@kyrylo kyrylo mentioned this issue Mar 11, 2015
Merged
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Mar 14, 2018
misc/ruby-pry: update to 0.11.3
2a9ca2d 
pkgsrc change: add support for pkg_alternatives

### HEAD

#### Features

* Add Pry::Testable, an improved modular replacement for PryTestHelpers.
  **breaking change**.

See pull request [#1679](pry/pry#1679).

* Add a new category module: "Pry::Platform". Loosely related to #1668 below.

See pull request [#1670](pry/pry#1670)

* Add `mac_osx?` and `linux?` utility functions to Pry::Helpers::BaseHelpers.

See pull request [#1668](pry/pry#1668).

* Add utility functions for drawing colorised text on a colorised background.

See pull request [#1673](pry/pry#1673).

#### Bug fixes

* Fix a case of infinite recursion in `Pry::Method::WeirdMethodLocator#find_method_in_superclass`
  that users of the [Hanami](http://hanamirb.org/) web framework experienced and
  reported since 2015.

See pull request [#1639](pry/pry#1689).

* Fix a bug where Method objects were not returned for setters inherited
  from a default (Pry::Config::Default). Eg, this is no longer an error:

      pry(main)> d = Pry::Config.from_hash({}, Pry::Config::Default.new)
      pry(main)> d.method(:exception_whitelist=) # Error

See pull request [#1688](pry/pry#1688).

* Do not capture unused Proc objects in Text helper methods `no_color` and `no_paging`,
  for performance reasons. Improve the documentation of both methods.

See pull request [#1691](pry/pry#1691).

* Fix `String#pp` output color.

See pull request [#1674](pry/pry#1674).

### 0.11.0

* Add alias 'whereami[?!]+' for 'whereami' command. ([#1597](pry/pry#1597))
* Improve Ruby 2.4 support ([#1611](pry/pry#1611)):
  * Deprecated constants are hidden from `ls` output by default, use the `-d` switch to see them.
  * Fix warnings that originate in Pry while using the repl.
* Improve completion speed in large applications. ([#1588](pry/pry#1588))
* Pry::ColorPrinter.pp: add `newline` argument and pass it on to PP. ([#1603](pry/pry#1603))
* Use `less` or system pager pager on MS Windows if it is available. ([#1512](pry/pry#1512))
* Add `Pry.configure` as an alternative to the current way of changing configuration options in `.pryrc` files. ([#1502](pry/pry#1502))
* Add `Pry::Config::Behavior#eager_load!` to add a possible workaround for issues like ([#1501](pry/pry#1501))
* Remove Slop as a runtime dependency by vendoring v3.4 as Pry::Slop.
  People can depend on Slop v4 and Pry at the same time without running into version conflicts. ([#1497](pry/pry#1497))
* Fix auto-indentation of code that uses a single-line rescue ([#1450](pry/pry#1450))
* Remove "Pry::Config#refresh", please use "Pry::Config#clear" instead.
* Defining a method called "ls" no longer breaks the "ls" command ([#1407](pry/pry#1407))
* Don't raise when directory permissions don't allow file expansion ([#1432](pry/pry#1432))
* Syntax highlight <tt> tags in documentation output.
* Add support for BasicObject subclasses who implement their own #inspect (#1341)
* Fix 'include RSpec::Matchers' at the top-level (#1277)
* Add 'gem-readme' command, prints the README file bundled with a rubygem
* Add 'gem-search' command, searches for a gem with the rubygems.org HTTP API
* Fixed bug in the `cat` command where it was impossible to use line numbers with files ([#1349](pry/pry#1349))
* Fixed uncaught Errno::EOPNOTSUPP exception when $stdout is a socket ([#1352](pry/pry#1352))
* Display a warning when you cd'ed inside a C object and executed 'show-source' without arguments ([#691](pry/pry#691))
* Make the stagger_output method more reliable by reusing possibly available Pry instance ([#1364](pry/pry#1364))
* Make the 'gem-install' message less confusing by removing backticks ([#1350](pry/pry#1350))
* Fixed error when Pry was trying to load incompatible versions of plugins ([#1312](pry/pry#1312))
* Fixed bug when `hist --clear` led to ArgumentError ([#1340](pry/pry#1340))
* Fixed the "uninitialized constant Pry::ObjectPath::StringScanner" exception during autocomplete ([#1330](pry/pry#1330))
* Secured usage of colours with special characters (RL_PROMPT_START_IGNORE and RL_PROMPT_END_IGNORE) in Pry::Helpers::Text ([#493](pry/pry#493 (comment)))
* Fixed regression with `pry -e` when it messes the terminal ([#1387](pry/pry#1387))
* Fixed regression with space prefixes of expressions ([#1369](pry/pry#1369))
* Introduced the new way to define hooks for commands (with `Pry.hooks.add_hook("{before,after}_commandName")`). The old way is deprecated, but still supported (with `Pry.commands.{before,after}_command`) ([#651](pry/pry#651))
* Removed old API's using `Pry::Hooks.from_hash` altogether
* Removed hints on Foreman support (see [this](ddollar/foreman#536))
* Fixed support for the tee command ([#1334](pry/pry#1334))
* Implemented support for CDPATH for ShellCommand ([#1433](pry/pry#1433), [#1434](pry/pry#1434))
* `Pry::CLI.parse_options` does not start Pry anymore ([#1393](pry/pry#1393))
* The gem uses CPU-less platforms for Windows now ([#1410](pry/pry#1410))
* Add `Pry::Config::Memoization` to make it easier to implement your own `Pry::Config::Default` class.([#1503](pry/pry#1503))
* Lazy load the config defaults for `Pry.config.history` and `Pry.config.gist`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
refactor
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging a pull request may close this issue.

Refactor command hooks so they could use Pry::Hooks (newer API) pry/pry
5 participants
@banister @moss @Mon-Ouie @ConradIrwin @rf-