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

Fix and Improve sql logging coloration in ActiveRecord::LogSubscriber. #20921

Merged
merged 2 commits into from Sep 9, 2015

Conversation

pboling
Copy link
Contributor

@pboling pboling commented Jul 17, 2015

  • Fixes coloring for SQL statements generated with Ruby heredoc, which often have spaces preceding the initial SQL verb, like:
   sql = <<-EOS
      SELECT * FROM THINGS
        WHERE ID IN (
          SELECT ID FROM THINGS
        )
   EOS

^ Would be colored as MAGENTA in the existing code.

  • Improves coloring for statements like:
    # Become WHITE
    SELECT * FROM (
        SELECT * FROM mytable FOR UPDATE
    ) ss WHERE col1 = 5;
    LOCK TABLE table_name IN ACCESS EXCLUSIVE MODE;

    # Becomes RED
    ROLLBACK
  • Reinstates the coloration of the payload[:name] via new method:
    colorize_payload_name
    Instead of simple alternating colors, adds meaning:
    • MAGENTA for "SQL" or blank? payload names
    • CYAN for Model Load/Exists
  • Make some ActiveRecord::LogSubscriber instance methods private for clarity:
    • colorize_payload_name
    • sql_color
    • logger
  • Introduces specs for sql coloration.
  • Introduces specs for payload name coloration.

GH #20885

- Improves coloring for statements like:

    # Become WHITE
    SELECT * FROM (
        SELECT * FROM mytable FOR UPDATE
    ) ss WHERE col1 = 5;
    LOCK TABLE table_name IN ACCESS EXCLUSIVE MODE;

    # Becomes RED
    ROLLBACK

- Reinstates the coloration of the `payload[:name]`.
  Instead of simple alternating colors, adds meaning:
  - `MAGENTA` for `"SQL"` or `blank?` payload names
  - `CYAN` for Model Load/Exists

- Introduces specs for sql coloration.
- Introduces specs for payload name coloration.

GH#20885
when /\s*\Adelete/i then RED
when /transaction\s*\Z/i then CYAN
else MAGENTA
when /\A\s*rollback/mi then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need then keywords if using multi-line case statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I am still used to writing code that is backwards compatible with Ruby vOLD. :P

Fixed.

@pboling pboling changed the title Improve sql logging coloration in ActiveRecord::LogSubscriber. Fix and Improve sql logging coloration in ActiveRecord::LogSubscriber. Jul 18, 2015
- CR feedback from @egilburg

Additionally

- Move logic for colorizing the payload name into a separate method
- Make some `ActiveRecord::LogSubscriber` instance methods private for clarity:
  - `colorize_payload_name`
  - `sql_color`
  - `logger`
- Improve Changelog Documentation

GH rails#20885
@rafaelfranca rafaelfranca added this to the 5.0.0 milestone Aug 31, 2015
@rafaelfranca rafaelfranca self-assigned this Aug 31, 2015
@rafaelfranca rafaelfranca merged commit 4fda1f2 into rails:master Sep 9, 2015
rafaelfranca added a commit that referenced this pull request Sep 9, 2015
Fix and Improve sql logging coloration in `ActiveRecord::LogSubscriber`.
@rafaelfranca rafaelfranca modified the milestones: 5.0.0 [temp], 5.0.0 Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants