search

gabe0x02 / log4js-json-layout

A Layout for log4js to output json lines
1 stars 0 forks source link

Question about sprintf implementation. #1

Open vogdb opened 8 years ago

vogdb commented 8 years ago

Hi! Thanks for the layout. Saved my time a lot.

Currently you have implemented sprintf like this:

      if(Array.isArray(to_json.data) && typeof to_json.data[0] === "string" ) {
        var count = (to_json.data[0].match(/%/g) || []).length;
        count -= (to_json.data[0].match(/%%/g) || []).length*2;
        var params = to_json.data.splice(0, count+1);           
        var str = sprintf.apply(this, params);
        to_json.data.splice(0, 0, str);
      }

Can we simplify it to?

      if(Array.isArray(to_json.data) && typeof to_json.data[0] === "string" && to_json.data[0].indexOf('%') >= 0) {
        to_json.data = sprintf.apply(this, to_json.data);
      }
vogdb commented 8 years ago

oh, first issue btw. 🎉

gabe0x02 commented 7 years ago

Hey vogdb, not sure if you are still using this. I'm currently in the process of thinking about logging issues again. I only just now saw you comments here. Sorry it's been over a year.

The original implementation is trying to only lose the values that being imbedded in output string so any additional arguments would still end up in the final JSON output. However your additional condition could increase performance in general.

vogdb commented 7 years ago

Well, the flag is on your side :) I left web development for now.