svg / svgo

āš™ļø Node.js tool for optimizing SVG files
https://svgo.dev/
MIT License
20.95k stars 1.39k forks source link

Preserve some newlines & tabs w/o --pretty #1300

Open TPS opened 3 years ago

TPS commented 3 years ago

N.B.: The closest issue I found for this is #230. If there's an existing option or open issue, I'd happily close this.

There needs to be a way to convert required, non-eliminable whitespace sequences made solely of

  1. (CR LF [other whitespace]) to just LF
  2. (Tab [non-{CR,LF} whitespace]) to just Tab
  3. (non-[CR,LF,Tab] whitespace) to Space (similar to current)
  4. Option: Convert some remaining spaces to LF to break up over-long (configurable) 1-liners efficiently

This will keep filesize the same as the current transform, while allowing to more easily compare by eye (or Github Ī” comparison šŸ˜‰) or edited by hand via any modern (i.e., basically anything newer than Windows Notepad šŸ™„) text editor. The sole downside I can think of that it might worsen SVGZ sizes for those who use that, but maybe an option to retain the current behavior would solve that.

--pretty has the problem that it generally increases filesize from minimal.

What do y'all think? I believe this would require to adjust https://www.github.com/svg/svgo/tree/master/plugins%2FcleanupAttrs.js to the above.

TPS commented 3 years ago

@dabutvin Just FYI, there was some pushback to adopting ImgBot w/o this RFE.

TPS commented 3 years ago

@dabutvin Maybe consider this for your SVGClean?

TrySound commented 3 years ago

I think we could add something like attrSep to js2svg config. Is this what you meant?

module.exports = {
  js2svg: {
    attrSep: '\n'
  }
};
<svg
xmlns="http://www.w3.org/2000/svg"
aria-label="Guidelines"
viewBox="0 0 512 512"><rect
width="512"
height="512"
rx="15%"
fill="#222"/><circle
fill="#f43"
r="256"
cx="256"
cy="256"/><circle
fill="#fd0"
r="217.5"
cx="256"
cy="256"/><circle
fill="#5d6"
r="192"
cx="256"
cy="256"/></svg>
TPS commented 3 years ago

That's not altogether bad, but here's what I mean (assuming original is https://github.com/edent/SuperTinyIcons/blob/master/images/guidelines/guideline.svg), which includes my option 4 above @ max 80 chars/line:

<svg xmlns="http://www.w3.org/2000/svg"
aria-label="Guidelines" role="img"
viewBox="0 0 512 512"><rect
width="512" height="512"
rx="15%"
fill="#222"/><circle fill="#f43" r="256" cx="256" cy="256"/><circle fill="#fd0"
r="217.5" cx="256" cy="256"/><circle fill="#5d6" r="192" cx="256"
cy="256"/></svg>

Better than yours for a compact diff, no?

TrySound commented 3 years ago

I wouldn't like to add such complexity in svg generator. Adding newlines to attributes may also be error prone. So better avoid new area of bugs.

TPS commented 3 years ago

Ok, leaving off LF in attributes, how about the rest of it?

TrySound commented 3 years ago

I don't get the rest

TPS commented 3 years ago

Sorry, I botched my example above. I updated it to be correct.

So, now, each line (separated by LF) corresponds more exactly with previous version (so changes seen better) but is optimal filesize (given what you pasted was already optimized), so diffs/visual inspection can show better what changed.

TPS commented 3 years ago

Here's a better IRL example: https://github.com/edent/SuperTinyIcons/pull/516/files

If using SVGO in default mode, the contents of adobe.svg would be mashed together in 1 line, & would be hard to differentiate what had changed. But, keeping whitespace (& attribute ordering, which already has an option) as close as possible to original, while keeping optimal filesize, allows the diff to be easily understood.

More examples @ https://github.com/edent/SuperTinyIcons/pull/517/files & https://github.com/edent/SuperTinyIcons/pull/518/files

TPS commented 3 years ago

I suppose this could be implemented as a mode of --pretty (in view of what you said above that only --pretty does anything with keeping any whitespace), but the aim of minimal bytesize is what makes the current --pretty not useful here.