implement indentation #80

oli-obk · 2016-04-16T14:03:37Z

fixes #79

This is a feature I'll definitely use in serde-xml, and probably implement it for json, too. Basically anything that processes tree-structures benefits enormously from it.

example run with miri:

Interpreting: factorial_recursive
 TRACE:miri::interpreter: // bb0
 TRACE:miri::interpreter: return = factorial_recursive::fact(const 10i64) -> bb1
  TRACE:miri::interpreter: // bb0
  TRACE:miri::interpreter: var0 = arg0
  TRACE:miri::interpreter: tmp1 = var0
  TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
  TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
  TRACE:miri::interpreter: // bb3
  TRACE:miri::interpreter: tmp2 = var0
  TRACE:miri::interpreter: tmp5 = var0
  TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
  TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
   TRACE:miri::interpreter: // bb0
   TRACE:miri::interpreter: var0 = arg0
   TRACE:miri::interpreter: tmp1 = var0
   TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
   TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
   TRACE:miri::interpreter: // bb3
   TRACE:miri::interpreter: tmp2 = var0
   TRACE:miri::interpreter: tmp5 = var0
   TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
   TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    TRACE:miri::interpreter: // bb0
    TRACE:miri::interpreter: var0 = arg0
    TRACE:miri::interpreter: tmp1 = var0
    TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    TRACE:miri::interpreter: // bb3
    TRACE:miri::interpreter: tmp2 = var0
    TRACE:miri::interpreter: tmp5 = var0
    TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    |TRACE:miri::interpreter: // bb0
    |TRACE:miri::interpreter: var0 = arg0
    |TRACE:miri::interpreter: tmp1 = var0
    |TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    |TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    |TRACE:miri::interpreter: // bb3
    |TRACE:miri::interpreter: tmp2 = var0
    |TRACE:miri::interpreter: tmp5 = var0
    |TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    |TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    | TRACE:miri::interpreter: // bb0
    | TRACE:miri::interpreter: var0 = arg0
    | TRACE:miri::interpreter: tmp1 = var0
    | TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    | TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    | TRACE:miri::interpreter: // bb3
    | TRACE:miri::interpreter: tmp2 = var0
    | TRACE:miri::interpreter: tmp5 = var0
    | TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    | TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    |  TRACE:miri::interpreter: // bb0
    |  TRACE:miri::interpreter: var0 = arg0
    |  TRACE:miri::interpreter: tmp1 = var0
    |  TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    |  TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    |  TRACE:miri::interpreter: // bb3
    |  TRACE:miri::interpreter: tmp2 = var0
    |  TRACE:miri::interpreter: tmp5 = var0
    |  TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    |  TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    |   TRACE:miri::interpreter: // bb0
    |   TRACE:miri::interpreter: var0 = arg0
    |   TRACE:miri::interpreter: tmp1 = var0
    |   TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    |   TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    |   TRACE:miri::interpreter: // bb3
    |   TRACE:miri::interpreter: tmp2 = var0
    |   TRACE:miri::interpreter: tmp5 = var0
    |   TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    |   TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    |    TRACE:miri::interpreter: // bb0
    |    TRACE:miri::interpreter: var0 = arg0
    |    TRACE:miri::interpreter: tmp1 = var0
    |    TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    |    TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    |    TRACE:miri::interpreter: // bb3
    |    TRACE:miri::interpreter: tmp2 = var0
    |    TRACE:miri::interpreter: tmp5 = var0
    |    TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    |    TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    |    |TRACE:miri::interpreter: // bb0
    |    |TRACE:miri::interpreter: var0 = arg0
    |    |TRACE:miri::interpreter: tmp1 = var0
    |    |TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    |    |TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    |    |TRACE:miri::interpreter: // bb3
    |    |TRACE:miri::interpreter: tmp2 = var0
    |    |TRACE:miri::interpreter: tmp5 = var0
    |    |TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    |    |TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    |    | TRACE:miri::interpreter: // bb0
    |    | TRACE:miri::interpreter: var0 = arg0
    |    | TRACE:miri::interpreter: tmp1 = var0
    |    | TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    |    | TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    |    | TRACE:miri::interpreter: // bb3
    |    | TRACE:miri::interpreter: tmp2 = var0
    |    | TRACE:miri::interpreter: tmp5 = var0
    |    | TRACE:miri::interpreter: tmp4 = Sub(tmp5, const 1i64)
    |    | TRACE:miri::interpreter: tmp3 = factorial_recursive::fact(tmp4) -> bb4
    |    |  TRACE:miri::interpreter: // bb0
    |    |  TRACE:miri::interpreter: var0 = arg0
    |    |  TRACE:miri::interpreter: tmp1 = var0
    |    |  TRACE:miri::interpreter: tmp0 = Eq(tmp1, const 0i64)
    |    |  TRACE:miri::interpreter: if(tmp0) -> [true: bb2, false: bb3]
    |    |  TRACE:miri::interpreter: // bb2
    |    |  TRACE:miri::interpreter: return = const 1i64
    |    |  TRACE:miri::interpreter: goto -> bb1
    |    |  TRACE:miri::interpreter: // bb1
    |    |  TRACE:miri::interpreter: return
    |    | TRACE:miri::interpreter: // bb4
    |    | TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    |    | TRACE:miri::interpreter: goto -> bb1
    |    | TRACE:miri::interpreter: // bb1
    |    | TRACE:miri::interpreter: return
    |    |TRACE:miri::interpreter: // bb4
    |    |TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    |    |TRACE:miri::interpreter: goto -> bb1
    |    |TRACE:miri::interpreter: // bb1
    |    |TRACE:miri::interpreter: return
    |    TRACE:miri::interpreter: // bb4
    |    TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    |    TRACE:miri::interpreter: goto -> bb1
    |    TRACE:miri::interpreter: // bb1
    |    TRACE:miri::interpreter: return
    |   TRACE:miri::interpreter: // bb4
    |   TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    |   TRACE:miri::interpreter: goto -> bb1
    |   TRACE:miri::interpreter: // bb1
    |   TRACE:miri::interpreter: return
    |  TRACE:miri::interpreter: // bb4
    |  TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    |  TRACE:miri::interpreter: goto -> bb1
    |  TRACE:miri::interpreter: // bb1
    |  TRACE:miri::interpreter: return
    | TRACE:miri::interpreter: // bb4
    | TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    | TRACE:miri::interpreter: goto -> bb1
    | TRACE:miri::interpreter: // bb1
    | TRACE:miri::interpreter: return
    |TRACE:miri::interpreter: // bb4
    |TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    |TRACE:miri::interpreter: goto -> bb1
    |TRACE:miri::interpreter: // bb1
    |TRACE:miri::interpreter: return
    TRACE:miri::interpreter: // bb4
    TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
    TRACE:miri::interpreter: goto -> bb1
    TRACE:miri::interpreter: // bb1
    TRACE:miri::interpreter: return
   TRACE:miri::interpreter: // bb4
   TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
   TRACE:miri::interpreter: goto -> bb1
   TRACE:miri::interpreter: // bb1
   TRACE:miri::interpreter: return
  TRACE:miri::interpreter: // bb4
  TRACE:miri::interpreter: return = Mul(tmp2, tmp3)
  TRACE:miri::interpreter: goto -> bb1
  TRACE:miri::interpreter: // bb1
  TRACE:miri::interpreter: return
 TRACE:miri::interpreter: // bb1
 TRACE:miri::interpreter: return

sfackler · 2016-04-16T15:35:24Z

Are there any existing logging frameworks that support a feature like this?

This seems to me like something that individual loggers (e.g. env-logger) would deal with themselves rather than giving log itself knowledge of indentation.

oli-obk · 2016-04-16T16:15:23Z

That would be possible, but then you need to have some kind of handle to the logger, which you can't really get without putting an arc into the logger

sfackler · 2016-04-16T16:44:31Z

static INDENT_LEVEL: AtomicUsize = ATOMIC_USIZE_INIT;

pub fn change_indent(l: usize) {
    INDENT_LEVEL.store(l, Ordering::SeqCst);
}

impl Log for IndentingLogger {
    fn log(&self, record: &LogRecord) {
        let level = INDENT_LEVEL.load(Ordering::SeqCst);
        // ...
    }
}

oli-obk · 2016-04-18T08:20:55Z

yea that would work, but then libraries need to know about the logger that's going to be used.

libraries set the indentation level, executables use it to print indented (or not if they don't care)

sfackler · 2016-04-18T16:29:18Z

My concern is that control over indentation is a very niche use case. Again, I am not aware of any logging framework that supports something like this.

In addition, having a single indentation level for the entire process seems like it'd have some serious issues with logging coming from multiple threads as they're all trying to first adjust the indentation and then log.

oli-obk · 2016-04-18T17:53:42Z

Ok, the thread thing is a serious issue. But we can fix that with a tls. Just because no logging framework has done it doesn't mean it's niche/rare. I've seen a few handwritten implementations. Whenever you have some sort of recursion or hierarchy, regular logging will become confusing. I have wished for indentation or something for a while when debugging rustc

xitep · 2016-04-18T18:02:01Z

this looks nice! though i see the usage limited to debugging/understanding a single threaded application with a console/file based logger. isn't the more general idea of what you really want sort of this?

oli-obk · 2016-04-19T13:52:43Z

@xitep yea, except that I don't want the logging statement to add the indentation, but the logger, so that even logging statements that don't know about indentation are indented. I doesn't matter to me whether it's numbers or indentation, that's a decision of the logger.

sfackler · 2016-04-20T16:40:05Z

I don't doubt that this is a thing you would find useful, but I also don't want this library to become the union of all features that anyone has ever found useful.

I would also note that rustc does not use this library, but rather its own version.

oli-obk · 2016-04-22T07:43:23Z

I would also note that rustc does not use this library, but rather its own version.

I know, but that is another issue.

I have an idea: A fn setting(name: &'static str, value: Box<Any+Send+'static>) (plus a macro that allows any library user to simply send a name + value pair to the logger). The logger can then choose to accept the setting depending on the name and whether the value is downcastable to the expected type).

This basically allows any kind of user extension, without cluttering the crate with additional features.

oli-obk · 2016-04-29T09:25:50Z

I updated this PR to a general "settings" version.

oli-obk · 2016-05-19T12:19:47Z

ping @sfackler

I also don't want this library to become the union of all features that anyone has ever found useful.

I can totally see that. The indentation part can be removed entirely, since it can be done in user code with the other changes.

The main question is whether settings of any kind should be allowed, and whether the design I proposed is feasible.

sfackler · 2016-05-19T16:56:03Z

I'm a bit concerned about identifying settings by string names - it seems like there's a high chance of naming conflicts between settings used by various logging backends, and of typos on the user side when interacting with them. Something type-based might be better? It might be too annoying to need to have to pull in some crate with a single struct in it though.

I'm still not sure that there will be a wide enough set of settings shared between the various logging frameworks that calling log::set_setting("indentation", 1) won't be effectively equivalent to extern crate env_logger; env_logger::set_indentation(1). This could potentially be prototyped in a separate crate - log_settings or something like that?

cc @rust-lang/libs

oli-obk · 2016-05-19T18:03:35Z

Good points, especially about separating out settings into an extra crate. I'll post here when I have something to show

brson · 2016-05-31T18:35:16Z

Can this be done in a logger implementation instead of directly in the log crate? Indentation is a property of the 'view' layer in my opinion, not the log lines themselves.

Just as an off-the cuff-idea, a custom logger crate could support a thread-local indentation level that was stored independently of the main logging information. So you might write something like:

indent_log(|| {
   info!(...);
})

oli-obk · 2016-05-31T18:39:26Z

I implemented it in the log_settings crate. Working on getting docs uploaded...

Indentation is a property of the 'view' layer in my opinion, not the log lines themselves.

I want other crates logs using the log crate to be indented, too. So the logger needs to read the value and crates that are indentation aware can change the indentation. Right now it's very rudimentary, but I'm considering adding something like you suggested.

dpc · 2017-05-08T18:56:25Z

@oli-obk Seems to me that you're hitting the ceiling of what's possible with non-structured logging. Have you seen slog's compact term logger format example (screenshot on https://github.com/slog-rs/slog)? Even if it's not exactly what you desire, you would have much easier time fitting your exact use-case into slog than log.

oli-obk · 2017-05-09T06:52:40Z

Cool! I'll have a look at it.

…w-1.0.51

oli-obk mentioned this pull request Apr 16, 2016

Allow box allocations rust-lang/miri#1

Closed

oli-obk closed this May 8, 2017

oli-obk force-pushed the master branch from 43a7508 to 4195412 Compare May 8, 2017 14:46

EFanZh pushed a commit to EFanZh/log that referenced this pull request Jul 23, 2023

Merge pull request rust-lang#80 from ryankurte/dependabot/cargo/anyho…

81cf2fc

…w-1.0.51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement indentation #80

implement indentation #80

oli-obk commented Apr 16, 2016 •

edited

Loading

sfackler commented Apr 16, 2016

oli-obk commented Apr 16, 2016

sfackler commented Apr 16, 2016

oli-obk commented Apr 18, 2016 •

edited

Loading

sfackler commented Apr 18, 2016

oli-obk commented Apr 18, 2016

xitep commented Apr 18, 2016

oli-obk commented Apr 19, 2016 •

edited

Loading

sfackler commented Apr 20, 2016

oli-obk commented Apr 22, 2016

oli-obk commented Apr 29, 2016

oli-obk commented May 19, 2016

sfackler commented May 19, 2016

oli-obk commented May 19, 2016

brson commented May 31, 2016

oli-obk commented May 31, 2016

dpc commented May 8, 2017

oli-obk commented May 9, 2017

implement indentation #80

implement indentation #80

Conversation

oli-obk commented Apr 16, 2016 • edited Loading

sfackler commented Apr 16, 2016

oli-obk commented Apr 16, 2016

sfackler commented Apr 16, 2016

oli-obk commented Apr 18, 2016 • edited Loading

sfackler commented Apr 18, 2016

oli-obk commented Apr 18, 2016

xitep commented Apr 18, 2016

oli-obk commented Apr 19, 2016 • edited Loading

sfackler commented Apr 20, 2016

oli-obk commented Apr 22, 2016

oli-obk commented Apr 29, 2016

oli-obk commented May 19, 2016

sfackler commented May 19, 2016

oli-obk commented May 19, 2016

brson commented May 31, 2016

oli-obk commented May 31, 2016

dpc commented May 8, 2017

oli-obk commented May 9, 2017

oli-obk commented Apr 16, 2016 •

edited

Loading

oli-obk commented Apr 18, 2016 •

edited

Loading

oli-obk commented Apr 19, 2016 •

edited

Loading