Ruby debugger corrections #1231

Merged
merged 2 commits into from May 23, 2011

Conversation

Projects
None yet
4 participants
Contributor

joshk commented May 23, 2011

Hey Guys,

Here are some minor corrections to ActiveSupport core_ext/kernel debugger related code, one which removes a require_library_or_gem, which is bad practice imo, and subsequent requires (which were unused). And another commit which removes AS removing breakpoint from Kernel if it existed, this is added by ruby-debug as an alias and I am not sure why Rails would say its been renamed.

I think this is good to apply to 3-1-stable and master, but I leave that up to you.

Thanks,

Josh

@vijaydev vijaydev and 1 other commented on an outdated diff May 23, 2011

activesupport/test/core_ext/kernel_test.rb
+ attr_reader :output
+ def puts(message)
+ @output ||= []
+ @output << message
+ end
+
+ def info(message)
+ puts(message)
+ end
+
+ def write(message)
+ puts(message)
+ end
+end
+
+class KernelDeuggerTest < Test::Unit::TestCase
@vijaydev

vijaydev May 23, 2011

Member

Typo in class name :)

@joshk

joshk May 23, 2011

Contributor

damn it!

thanks for catching that

one sec ...

@joshk

joshk May 23, 2011

Contributor

fixed, thanks :)

@joshk joshk Removed the AS core_ext/kernel/debugger breakpoint method as this met…
…hod is added by ruby-debug and should be left as is. Missing tests added.
2514d65

josevalim merged commit a5fb1c6 into rails:master May 23, 2011

Owner

fxn commented on eff5d9a May 23, 2011

Hey, there are still references to this method in the project. Could you perform another grep?

Contributor

joshk replied May 23, 2011

Are there? I'm sorry, I will check into it further, my ack didn't pick anything up.

Owner

fxn replied May 23, 2011

You're not the first one to fall there, I promise you :).

Thing is guides are written in Textile, and ack ignores .textile files by default. You need to pass -a to have them ack'ed :).

Contributor

joshk replied May 23, 2011

ahhhhhhhhh, so require_library_or_gem is referenced in the guides? I am using the textmate ack bundle, not sure how to tell it to include textile, I'll look into it and get this updated.

Thanks for letting me know.

Member

vijaydev replied May 23, 2011

@fxn I just tried out using ack and it did search through the textile files. @joshk: Check out Active Support Core Extensions Guide! :)

Contributor

joshk replied May 23, 2011

I have committed the correction to docrails

rails/docrails@09314f2

Contributor

joshk replied May 23, 2011

@vijaydec i fixed my ackmate textmate plugin to include textile for now on

Owner

fxn replied May 23, 2011

@vijaydev could it be the case that you've got a custom ~/.ackrc ?

Member

vijaydev replied May 24, 2011

@fxn No, I don't have one.

Owner

fxn replied May 24, 2011

That's strange. The default types do not include .textile

https://github.com/petdance/ack/blob/master/Ack.pm#L74-128

What does the command

ack --help=types | ack textile

output in your machine?

Member

vijaydev replied May 24, 2011

Hmm... the command outputs nothing. Meaning textile is not a supported type?!

ack-grep version: 1.92

Owner

fxn replied May 24, 2011

That's the expected output. I have 1.94 here, but .textile has never been among the types known by ack by default. Errrr, can't understand why is searching them in your machine.

What about environment variables? For example,

env | ack ACK

Member

vijaydev replied May 24, 2011

Nothing in env either!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment