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

Multi-Document-Separators "#---" comment checks do not account for '!' prefixed comments #32185

Closed
PaulKlumpp opened this issue Aug 29, 2022 · 2 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@PaulKlumpp
Copy link

PaulKlumpp commented Aug 29, 2022

Using Spring Boot 2.7.3. The docs for Spring Boot 2.7.3 say on https://docs.spring.io/spring-boot/docs/2.7.3/reference/htmlsingle/#features.external-config.files.multi-document

The lines immediately before and after the separator must not be comments.

Using IDEs like IntelliJ, often, blank lines get removed. It is a setting for "Code Style" on "properties" files.

When the IDE removed a blank line, it happened. A comment from the line above got directly placed in front of the document separator line containing "#---".

Like this, the multi document was not recognized anymore and led to problems like a misconfiguration of the application.

Sure, we will activate the Code Style Feature "Keep blank lines" for "properties", but still... Since comments should be perfectly fine in a property-file, it feels like a parser bug that it isn't allowed to have a comment line in front of and after the "#---" line.

Please make it valid to have a multi-document like the following:

  • When the "testing" profile is active, myProp=abcdef should be set.
  • When the "testing" profile is not active, myProp should not be set.
#======= eye-sight-separator =======
#---
spring.config.activate.on-profile=testing
myProp=abcdef

With 2.7.3, when we did not activate the "testing" profile, myProp=abcdef was set. Ouch.

At the very moment, only the following works correctly:

#======= eye-sight-separator =======

#---
spring.config.activate.on-profile=testing
myProp=abcdef

To add a little more of an info..
I think the problem might be in here: https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedPropertiesLoader.java

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 29, 2022
@philwebb
Copy link
Member

philwebb commented Sep 1, 2022

Some history about why we added this restriction can be found in #22963. If we remove it then we're likely to break folks that have comments such as:

#----
# Some comment
#----

In this example, the user doesn't want #--- to be treated as a separator.

! is also a valid comment line so I wonder if we can extend our existing support to also include !--- and change the rule so that the line before must not have the same comment prefix. In other words, we'd allow:

#======= eye-sight-separator =======
!---
spring.config.activate.on-profile=testing
myProp=abcdef

I think the existing code only considers # when checking the previous line, so you might already be able to do this:

!======= eye-sight-separator =======
#---
spring.config.activate.on-profile=testing
myProp=abcdef

@philwebb philwebb changed the title Multi-Document-Separators "#---": The lines immediately before and after the separator must not be comments. Multi-Document-Separators "#---" comment checks do not account for '!' prefixed comments Sep 1, 2022
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 1, 2022
@philwebb philwebb added this to the 2.6.x milestone Sep 1, 2022
terminux added a commit to terminux/spring-boot that referenced this issue Sep 27, 2022
@philwebb
Copy link
Member

Closing in favor of PR #32521. Thanks @terminux!

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Sep 29, 2022
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed type: bug A general bug labels Sep 29, 2022
@philwebb philwebb removed this from the 2.6.x milestone Sep 29, 2022
terminux added a commit to terminux/spring-boot that referenced this issue Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

3 participants