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

Fix editor live validation #6431

Merged
merged 1 commit into from Mar 26, 2024
Merged

Fix editor live validation #6431

merged 1 commit into from Mar 26, 2024

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Mar 26, 2024

What does this implement/fix?

This broke in 25ab6f0 because read_config_file was no longer being called. It was missed that read_config_file will check the CORE.vscode and CORE.ace globals, and in some cases instead of reading a config file from disk, it will print an event to stdout, and than decode json from stdin to get the contents of the yaml.

def read_config_file(path: str) -> str:
    if CORE.vscode and (
        not CORE.ace or os.path.abspath(path) == os.path.abspath(CORE.config_path)
    ):

This PR removes read_config_file since its no longer used, which removes more of the global CORE.ace and CORE.vscode checks.

There are still two places where these global checks remain that should be removed, but they are a bit harder to remove and not contemplated in this PR

esphome/config_validation.py:        not CORE.ace or os.path.abspath(path) == os.path.abspath(CORE.config_path)
esphome/config_validation.py:        not CORE.ace or os.path.abspath(path) == os.path.abspath(CORE.config_path)

I added some more typing since it was very hard to follow what was going on as the variables names were not obvious.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes esphome/issues#5533

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

# Example config.yaml

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@bdraco bdraco force-pushed the fix_vs_code_validation branch 2 times, most recently from d3b22ad to b543a6f Compare March 26, 2024 05:16
)
)


def read_config(args):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add tests for this, but in its current form its not very testable, and I didn't want to refactor it here.

@bdraco bdraco marked this pull request as ready for review March 26, 2024 05:42
@bdraco bdraco requested a review from a team as a code owner March 26, 2024 05:42
@bdraco bdraco added this to the 2024.3.1 milestone Mar 26, 2024
@jesserockz jesserockz merged commit 2345e76 into dev Mar 26, 2024
53 checks passed
@jesserockz jesserockz deleted the fix_vs_code_validation branch March 26, 2024 07:24
jesserockz pushed a commit that referenced this pull request Mar 26, 2024
@jesserockz jesserockz mentioned this pull request Mar 26, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor Code Check not 'Live' anymore [Edit: Being worked on]
2 participants