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

Use uniform spacing and (LF-only) line endings #283

Closed
dandv opened this Issue Oct 5, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@dandv

dandv commented Oct 5, 2016

RFG uses a mix of spaces, tabs, LF, and CR+LF line separators:

  • browser.config.xml is CRL+LF-separated. Arguably this is because it targets Windows, but the file likely won't be edited on Windows, and there are other text files in the package that Notepad will display in one lines. The XML uses spaces for indentation - great.
  • manifest.json uses LF-only line separators, but tabs instead of spaces for indentation
  • safari-pinned-tab uses LF line endings and wraps at 74 chars/line.

@phbernard phbernard added this to the Package v0.14 milestone Oct 6, 2016

@phbernard

This comment has been minimized.

Show comment
Hide comment
@phbernard

phbernard Oct 6, 2016

Contributor

Thanks. I never realized this.

So:

  • manifest.json looks okay. This is what json_encode and JSON_PRETTY_PRINT generate so I rather stick to it.
  • browserconfig.xml: let's switch to LF-only and tabs, to match manifest.json. Plus it's (a little bit) more compact.
  • Safari pinned tabs SVG icon: SVGO will decide, see #261.
Contributor

phbernard commented Oct 6, 2016

Thanks. I never realized this.

So:

  • manifest.json looks okay. This is what json_encode and JSON_PRETTY_PRINT generate so I rather stick to it.
  • browserconfig.xml: let's switch to LF-only and tabs, to match manifest.json. Plus it's (a little bit) more compact.
  • Safari pinned tabs SVG icon: SVGO will decide, see #261.
@phbernard

This comment has been minimized.

Show comment
Hide comment
@phbernard

phbernard Nov 3, 2016

Contributor

Fixed in branch package_v0_14

Contributor

phbernard commented Nov 3, 2016

Fixed in branch package_v0_14

@phbernard

This comment has been minimized.

Show comment
Hide comment
@phbernard

phbernard Jan 18, 2017

Contributor

Deployed a minute ago.

Contributor

phbernard commented Jan 18, 2017

Deployed a minute ago.

@phbernard phbernard closed this Jan 18, 2017

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Jan 18, 2017

@phbernard: nitpicking but manifest.json does not have the end of file line break like the browserconfig.xml has. :)

XhmikosR commented Jan 18, 2017

@phbernard: nitpicking but manifest.json does not have the end of file line break like the browserconfig.xml has. :)

@phbernard

This comment has been minimized.

Show comment
Hide comment
@phbernard

phbernard Jan 18, 2017

Contributor

Hex editor speaking :) Both manifest.json and browserconfig.xml have UNIX-style (ie. 0x0A) end of line.

Do you observe something else?

Contributor

phbernard commented Jan 18, 2017

Hex editor speaking :) Both manifest.json and browserconfig.xml have UNIX-style (ie. 0x0A) end of line.

Do you observe something else?

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Jan 18, 2017

XhmikosR commented Jan 18, 2017

@phbernard

This comment has been minimized.

Show comment
Hide comment
@phbernard

phbernard Jan 19, 2017

Contributor

I've just tested it again. I clearly see 0x0a everywhere (and not 0x0d0a):

JSON

image

XML

image

Could you tell me how you check the end of line?

Contributor

phbernard commented Jan 19, 2017

I've just tested it again. I clearly see 0x0a everywhere (and not 0x0d0a):

JSON

image

XML

image

Could you tell me how you check the end of line?

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Jan 19, 2017

It's obvious for me just by opening the files...

See the attached images.

browserconfig

manifest

As you can see manifest.json doesn't have the end of file line break.

XhmikosR commented Jan 19, 2017

It's obvious for me just by opening the files...

See the attached images.

browserconfig

manifest

As you can see manifest.json doesn't have the end of file line break.

@phbernard

This comment has been minimized.

Show comment
Hide comment
@phbernard

phbernard Jan 19, 2017

Contributor

Oh, okay! The final line break at the end of the file! Sorry, I misread your first message, I thought you were talking about all ends of line.

I consider this as another issue, although it would have been good to fix everything at once. I just entered #304

Contributor

phbernard commented Jan 19, 2017

Oh, okay! The final line break at the end of the file! Sorry, I misread your first message, I thought you were talking about all ends of line.

I consider this as another issue, although it would have been good to fix everything at once. I just entered #304

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment