Coalesce multiline logs into one line

RESOLVED FIXED in Firefox 61

Status

enhancement
P1
normal
RESOLVED FIXED
Last year
7 months ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

Trunk
mozilla61
All
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(3 attachments)

To make it easier to wrap a long log line into multiple lines, we should coalesce multiline literals back into one line. E.g., turn

> debug `foo=${foo}
>        bar=${bar}`

into

> debug `foo=${foo} bar=${bar}`

And use

> debug `foo=${foo}`
> debug `bar=${bar}`

For actually logging multiple lines.
Comment on attachment 8969074 [details]
Bug 1455112 - 1. Coalesce multiple log lines into one;

https://reviewboard.mozilla.org/r/237760/#review243738

Nice, thanks!

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:329
(Diff revision 1)
>     *   do_something(debug.foo = bar); // Output "foo = 42" to the log.
>     *
>     * @param tag Name of the Log.jsm logger to forward logs to.
>     * @param scope Scope to add the logging functions to.
>     */
>    initLogging: function(tag, scope) {

aTag, aScope.

I somewhat regret our decision to go with prefixes in JS.

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:358
(Diff revision 1)
>        this._rootLogger.addAppender(new Log.AndroidAppender());
>      }
>      return this._rootLogger;
>    },
>  
>    _log: function(logger, level, strings, exprs) {

a-prefixes.

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:369
(Diff revision 1)
>      }
>  
> +    if (logger.level > Log.Level.Numbers[level]) {
> +      // Log disabled.
> +      return;
> +    }

That's a different bug, isn't it?
Attachment #8969074 - Flags: review?(esawin) → review+
Comment on attachment 8969075 [details]
Bug 1455112 - 2. Add eslint rules to enforce debug/warn usage;

https://reviewboard.mozilla.org/r/237762/#review243746

LGTM but I don't know anything about eslint config.
Attachment #8969075 - Flags: review?(esawin) → review+
Priority: -- → P1
Comment on attachment 8969370 [details]
Bug 1455112 - 3. Update existing multiline debug lines;

https://reviewboard.mozilla.org/r/238120/#review243844
Attachment #8969370 - Flags: review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2cd06fff80fe
1. Coalesce multiple log lines into one; r=esawin
https://hg.mozilla.org/integration/autoland/rev/345d021363fb
2. Add eslint rules to enforce debug/warn usage; r=esawin
https://hg.mozilla.org/integration/autoland/rev/9c43c32dc98e
3. Update existing multiline debug lines; r=jchen
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 61 → mozilla61
You need to log in before you can comment on or make changes to this bug.