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

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

Open
TPS opened this issue Nov 14, 2020 · 10 comments
Open

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

TPS opened this issue Nov 14, 2020 · 10 comments

Comments

@TPS
Copy link

TPS commented Nov 14, 2020

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://github.com/svg/svgo/tree/master/plugins%2FcleanupAttrs.js to the above.

@TPS TPS changed the title Preserve newlines & tab w/o Pretty Preserve newlines & tab w/o --Pretty Nov 14, 2020
@TPS TPS changed the title Preserve newlines & tab w/o --Pretty Preserve newlines & tab w/o --pretty Nov 14, 2020
@TPS
Copy link
Author

TPS commented Nov 14, 2020

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

@TPS TPS changed the title Preserve newlines & tab w/o --pretty Preserve newlines & tabs w/o --pretty Nov 14, 2020
@TPS TPS changed the title Preserve newlines & tabs w/o --pretty Preserve some newlines & tabs w/o --pretty Nov 16, 2020
@TPS
Copy link
Author

TPS commented Dec 2, 2020

@dabutvin Maybe consider this for your SVGClean?

@TrySound
Copy link
Member

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
Copy link
Author

TPS commented Feb 19, 2021

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
Copy link
Member

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
Copy link
Author

TPS commented Feb 20, 2021

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

@TrySound
Copy link
Member

I don't get the rest

  • CR LF -> LF - there is no newlines with disabled pretty mode
  • Tab -> Tab - ??
  • (non-[CR,LF,Tab] whitespace) -> Space - what is different from now?

@TPS
Copy link
Author

TPS commented Feb 20, 2021

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
Copy link
Author

TPS commented Feb 20, 2021

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
Copy link
Author

TPS commented Feb 26, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants