Feature #5096

offer Logger-compatibility for syslog ext

Added by Eric Wong over 2 years ago. Updated almost 2 years ago.

[ruby-core:38503]
Status:Closed
Priority:Low
Assignee:Akinori MUSHA
Category:-
Target version:-

Description

There's http://rubygems.org/gems/SyslogLogger, but I would like to have
ths compatibility by default. Having to remember to install the
oddly-capitalized SyslogLogger gem makes things harder than it should
be.

Importing the SyslogLogger gem would solve this issue.

sysloglogger.patch Magnifier (19.6 KB) Aaron Patterson, 03/09/2012 07:24 AM

syslog.logger.patch Magnifier - rails-free patch (20.2 KB) Eric Hodel, 05/16/2012 09:59 AM

knu-syslog_logger.patch.txt Magnifier (5.67 KB) Akinori MUSHA, 05/17/2012 09:59 PM

Associated revisions

Revision 35682
Added by Eric Hodel almost 2 years ago

  • ext/syslog/lib/syslog/logger.rb: Added Syslog::Logger which was ported from the SyslogLogger gem. [ruby-trunk - Feature #5096]
  • NEWS: ditto.
  • test/syslog/testsysloglogger.rb: ditto.

History

#1 Updated by Eric Wong over 2 years ago

Bah, I submitted the bug before I finished typing :<

Title should be: "offer Logger-compatibility for syslog ext"
Priority should be: Low

There's http://rubygems.org/gems/SyslogLogger, but I would like to have
ths compatibility by default. Having to remember to install the
oddly-capitalized SyslogLogger gem makes things harder than it should
be.

Importing the SyslogLogger gem would solve this issue.

#2 Updated by Hiroshi Nakamura over 2 years ago

  • Subject changed from offer Logger-compatibility for ext to offer Logger-compatibility for syslog ext
  • Priority changed from Normal to Low

#3 Updated by Eric Hodel over 2 years ago

I would be happy to import my SyslogLogger gem, but I need someone to decide if it is ok first.

#4 Updated by Yui NARUSE over 2 years ago

Where is a patch or the repository?

Anyway such enhancement should be introduced as a patch to trunk.
We are hard to understand the actual change of this feature.

Moreover adding a new library or replacing a library is hard to accepted.
Enhance a existed library is much easier then it.

#5 Updated by Eric Hodel about 2 years ago

  • Assignee set to Eric Hodel

The repository is here: https://github.com/seattlerb/sysloglogger

I will attach a patch to this issue soon.

#6 Updated by Aaron Patterson about 2 years ago

I've attached a patch. It imports sysloglogger as Syslog::Logger. Syslog::Logger is a logger that logs to Syslog, but has the same API as Logger.

#7 Updated by Eric Hodel about 2 years ago

I think the implementation is fine, but the Syslog::Logger constant should be updated since the name was changed.

Also, should the rails references be removed or changed?

#8 Updated by Eric Hodel about 2 years ago

Ugh, I meant "Syslog::Logger comment"

#9 Updated by Shyouhei Urabe about 2 years ago

  • Status changed from Open to Assigned

#10 Updated by Eric Hodel almost 2 years ago

This updated patch removes references to rails and the silence method (as it is not thread-safe).

#11 Updated by Akinori MUSHA almost 2 years ago

  • Assignee changed from Eric Hodel to Akinori MUSHA

I'll look into this shortly.

I think I'm going to put this under ext/syslog/lib/syslog/ so the whole syslog library can be made a gem in the future.

#12 Updated by Eric Hodel almost 2 years ago

The second rails-free patch places it under ext/syslog/lib/syslog and introduces test/syslog/.

I did not move test/testsyslog.rb to test/syslog/testsyslog.rb though, I was going to submit this as a separate issue if this was was accepted.

#13 Updated by Akinori MUSHA almost 2 years ago

Oh, of course. I didn't carefully read the latest patch.

I've revised the implementation and the tests as attached.

Here's the list of what I changed:

  • Reduced map constants. You shouldn't need this many.

  • Changed Syslog::Logger::SYSLOG to Syslog::Logger.syslog using a class variable as storage. I didn't like the way the constant was used.

  • Changed to call syslog methods with ('%s', message) instead of manually replacing %'s.

  • Changed a regexp so it exactly matches (ANSI & XTerm) color escape sequences.

Please merge it if it's OK, and you have my approval.

#14 Updated by Eric Hodel almost 2 years ago

I think you forgot to attach the patch.

Your changes sound good, thank you for the review and update on my very (5 year) old code.

#16 Updated by Akinori MUSHA almost 2 years ago

D'oh, here it is.

#17 Updated by Eric Hodel almost 2 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r35682.
Eric, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • ext/syslog/lib/syslog/logger.rb: Added Syslog::Logger which was ported from the SyslogLogger gem. [ruby-trunk - Feature #5096]
  • NEWS: ditto.
  • test/syslog/testsysloglogger.rb: ditto.

Also available in: Atom PDF