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

Improvements in line breaks #15

Open
murdos opened this issue Oct 15, 2023 · 3 comments
Open

Improvements in line breaks #15

murdos opened this issue Oct 15, 2023 · 3 comments

Comments

@murdos
Copy link

murdos commented Oct 15, 2023

Using version 0.3.0, with printWidth: 140 and keySeparator: '=' configuration.

I'm not sure what rules could be inferred, but on these examples the properties before transformation are easier to read.

Before

sonar.cpd.exclusions=\
  src/main/java/tech/jhipster/lite/error/infrastructure/primary/ArgumentsReplacer.java,\
  src/main/java/tech/jhipster/lite/module/domain/file/ArgumentsReplacer.java

After

sonar.cpd.exclusions=src/main/java/tech/jhipster/lite/error/infrastructure/primary/ArgumentsReplacer.\
  java,src/main/java/tech/jhipster/lite/module/domain/file/ArgumentsReplacer.java

Another example:

Before

sonar.exclusions=\
  src/main/webapp/main.ts, \
  src/main/webapp/app/main.ts, \
  src/main/webapp/content/**/*.*, \
  src/main/webapp/i18n/*.js, \
  target/classes/static/**/*.*, \
  src/main/resources/**, \
  src/main/webapp/app/router/index.ts, \
  src/main/glyph/css/**, \
  src/main/webapp/app/common/primary/applicationlistener/WindowApplicationListener.ts

After

sonar.exclusions=src/main/webapp/main.ts, src/main/webapp/app/main.ts, src/main/webapp/content/**/*.*, src/main/webapp/i18n/*.js, \
  target/classes/static/**/*.*, src/main/resources/**, src/main/webapp/app/router/index.ts, src/main/glyph/css/**, \
  src/main/webapp/app/common/primary/applicationlistener/WindowApplicationListener.ts

Another example:

Before

assertion-error.STRING_TOO_LONG.detail=\
  Content of {{ field }} is too long, must be less than {{ maxLength }} character(s) (was {{ currentLength }})

After

assertion-error.STRING_TOO_LONG.detail=Content of {{ field }} is too long, must be less than {{ maxLength }} character(s) (was {{ \
  currentLength }})
@eemeli
Copy link
Owner

eemeli commented Oct 16, 2023

Yeah, those are hard. One improvement that they do suggest to me is that when a value does not fit on the same line with the key, it might make sense to always (?) start it on the next line instead, using an escaped newline.

Or are there cases where that would be worse choice than the current one?

@murdos
Copy link
Author

murdos commented Oct 16, 2023

Yes, always starting on the next line when a value does not fit on the same line with the key sounds like a good and safe idea.

I was also thinking about other heuristics, but I'm not sure they always apply:

  • split to a new line after each comma ,
  • avoid splitting a "word" around a dot . if there's no space after of before the dot. This should avoid split e.g.
    • src/main/webapp/app/router/index.ts to src/main/webapp/app/router/index. and ts.
    • or https://github.com/eemeli/prettier-plugin-properties to https://github. and com/eemeli/prettier-plugin-properties

@eemeli
Copy link
Owner

eemeli commented Oct 17, 2023

split to a new line after each comma ,

This would not really work for property values that are e.g. localizable text.

avoid splitting a "word" around a dot . if there's no space after of before the dot.

In practice that would probably mean dropping this line, which should be pretty safe.

Or maybe replacing it with case ',':?

Note that the changes under consideration are required in dot-properties, rather than this repo.

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