- Notifications
You must be signed in to change notification settings - Fork380
Scalafmt two scalalib/overrides* files#4605
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Scalafmt two scalalib/overrides* files#4605
Conversation
| *@example {{{ | ||
| * object Main extends App { | ||
| *@example | ||
| * {{{object Main extends App { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This area looks like it shouldn't have been changed with the{{{ ... }}} block but maybe there was a formatting issue to begin with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you for noticing the difference. I used the repositoryscripts/scalafmt with no local
modifications. That should have used the JVM variant of scalafmt with the preferred
.scalafmt. Easy peasy!
Your thoughts? Beyond "silly person, why did you touch this in the first place?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
all scaladoc tags (@example in this case) are formatted with [some of] their parameters on the same line, but anything that is not a parameter (for instance, body of the tag) is formatted on the next line. tags with parameters are, for example,@param and@throws.
A possible least-effort/greatest benefit solution is to let these two files merge All the rest of the files currently in the repository are formatted. Dissonance Is there a way to drop a .scalafmt in that directory which matches upstream? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@LeeTibbert you should abandon, since@WojciechMazur said we shouldn't be formatting scalalib.
I think I know why you saw these files re-formatted. I will submit a PR to fix it.
| *@example {{{ | ||
| * object Main extends App { | ||
| *@example | ||
| * {{{object Main extends App { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
all scaladoc tags (@example in this case) are formatted with [some of] their parameters on the same line, but anything that is not a parameter (for instance, body of the tag) is formatted on the next line. tags with parameters are, for example,@param and@throws.
The issue is more likekly the exclude pattern in scalafmt config I belive it should be If you've cloned the repo under different name it would not match the glob. |
Status: I am leaving this Issue open until I have done a few private design studies for other Issues. Those |
Uh oh!
There was an error while loading.Please reload this page.
Prior to this PR running
scripts/scalafmtorcheck-lint.shon a freshly fetched copy of the SN Repositoryin preparation for submitting a PR would show two
scalalib/override*files as modified.This PR allows a 'round trip' of fetching the SN repository, making a few point changes, and then
pushing back to the SN repository as a PR.
Reviewers
There may be a reason, such as the files being copied 'as-is' from somewhere else in the Scala
development environment and wanting to minimize non-semantic changes. In that case,
it might be able to list them in a .gitignore.
Later:
.gitignoreand it has a linescalalib/src/but nothing I could see forscalalib/overrides.Perhaps adding such is the preferred solution?
I would have to determine the proper syntax for
scalalib/overridesand below.Small Matter of Programming.
Later:
Right "Small", everything is SMOP until you are the one doing it ;-)
If discovered/re-learned that adding
scalalib/overrideswill indeed ignorethat directory and its descendants. Good so far.
The difficulty seems to be that files at or below the just added line
that are already in the Repository will not be ignored. Fixing things
up seems to involve saving a copy of the file, deleting it from the repository,
then adding it back in. Error prone surgery to a critical area.
During CI wait time I'll rat around the GitHub docs and the wider Web
to see if there is a less anxiety producing solution, such as renaming.
SMOP, Right!
Please advise as time allows.