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: default config overwrite #719

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

nemanja-m
Copy link
Contributor

This PR fixes #718.

After the v1.61.4 release, the user-specified config is overwritten by default values

This commit introduced a bug:

c8498c2#diff-19e19f16fa5018b7ad46fc960cdbe0138b0029dac020b42c798a9ff9b8203493R14


Initial configuration is updated with command line args:

https://github.com/air-verse/air/blob/master/main.go#L97

but, when defaultConfig is used in

https://github.com/air-verse/air/blob/master/runner/flag.go#L9-L17

all command line values are set to default, effectively overwriting everything from user-specified .air.toml.

This PR ensures that only command-line arguments that differ from default values overwrite the config. This is safe since the initial config is based on the default and the user’s .air.toml.

Copy link

@PThorpe92 PThorpe92 left a comment

Choose a reason for hiding this comment

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

haha you beat me to it. LGTM 👍

@xiantang
Copy link
Collaborator

im checking

Copy link

codecov bot commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
runner/config.go 76.47% <100.00%> (+0.19%) ⬆️
runner/flag.go 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

@xiantang xiantang merged commit 0232344 into air-verse:master Dec 17, 2024
8 checks passed
wafer-bw added a commit to searchspring/air that referenced this pull request Jan 31, 2025
* chore: bump go to 1.23 (air-verse#650)

* docs: update banner (air-verse#651)

* fix proxy retry loop (air-verse#635)

This addresses two bugs:
1) The loop could exit with an error condition present and fail to
   inform the user.
2) On Windows 11, syscall.ECONNREFUSED is not returned. Instead we
   receive net.OpError with a message similar to:

"unable to reach app: Get "http://localhost:7000/": dial tcp [::1]:7000: connectex: No connection could be made because the target machine actively refused it."

The Fix:
Since the retry loop is short; just 1 second maximum, we retry until no
error happens. If an error does occur, we inform the user.

* Use web page address when making request to reload proxy (air-verse#625)

* Use web page address when making request to reload proxy

Signed-off-by: Alexandra Fishova <[email protected]>

* Create event source with just a path because browser can handle address on its own.

Signed-off-by: Alexandra Fishova <[email protected]>

---------

Signed-off-by: Alexandra Fishova <[email protected]>

* fix the stop_on_error is false configuration does not work (air-verse#555)

* fix color output (air-verse#551)

* Set the Via header in proxy requests to origin and in client responses (air-verse#629)

* Set the Via header in proxy requests to origin and in client responses

* Remove pseudonym and use host only

* feat: Make include_file overrule include_ext, fixes: air-verse#350, base_on: air-verse#358 (air-verse#416)

* feat: Make include_file overrule include_ext

* feat: Make include_file overrule include_ext unittest

* fix: broken code (air-verse#652)

* feat: silent flag in .air.toml, suppressing all air prints (air-verse#641)

* feat: silent flag in .air.toml, suppressing all air prints

useful if the app being watched has structured outputs that are being
piped into subsequent program/s.

* update: add silent to air_example.toml

* FIX: Support working directories containing whitespace (air-verse#646)

* encapsulate paths with quotes to escape spaces

* encapsulate paths with quotes to escape spaces

* preprocess error in test

* Revert "FIX: Support working directories containing whitespace (air-verse#646)" (air-verse#665)

This reverts commit df13da5.

* refactor: remove unnecessary var copy in for (air-verse#682)

* docs: fix badge link in README-zh_cn.md (air-verse#681)

* refactor: simplify test asserts; enable testifylint (air-verse#680)

* Update unclear .profile changes in README.md (air-verse#684)

* Fix: typo in air_example.toml (air-verse#706)

* Implement kill delay on Windows (air-verse#552)

* fix color output

* Implement kill delay on Windows

* Revert the change on master

* Remove extra newline

* Handle absolute paths in config (air-verse#318)

* remove a data race by compiling the exclude regexes in the preprocess phase (air-verse#677)

the regex compilation was done lazily on first access but did not properly synchronize
for being accessed by multiple watcher goroutines

between the option of adding a mutex for something that (should) only ever happen
once and removing the potential for a race, this seems like the better choice

* catch and report start cmd failure rather than panicing (air-verse#676)

* do not report the process as running if it failed to start

* change the return values on error to match what the other platforms produce

* chore: fix test case TestRegexes

* air -h will be document of air

* Update README for air help command

* fix: default config override (air-verse#719)

* Avoid duplicating cors header. (air-verse#727)

* fix: typos (air-verse#728)

* wait for killfunc completion when shutting down current app (air-verse#670)

* wait for killfunc completion when shutting down current app

---------

Co-authored-by: jingdi.zhu <[email protected]>

* Feature: Show build errors when using proxy (air-verse#725)

* proxy: stream reload and error messages

* proxy: Console log on build failure

* proxy: show build errors in a modal

---------

Co-authored-by: xiantang <[email protected]>

* Add exiter to test os.Exit case (air-verse#729)

authored-by: jingdi.zhu <[email protected]>

* Fix: using powershell instead of cmd /C (air-verse#708)

---------

Signed-off-by: Alexandra Fishova <[email protected]>
Co-authored-by: Rick Yu <[email protected]>
Co-authored-by: Greg Dietsche <[email protected]>
Co-authored-by: Alexandra Fishova <[email protected]>
Co-authored-by: dengdajun <[email protected]>
Co-authored-by: 8LWXpg <[email protected]>
Co-authored-by: dedalusj <[email protected]>
Co-authored-by: zhb127 <[email protected]>
Co-authored-by: ~ jesse ~ <[email protected]>
Co-authored-by: osteensco <[email protected]>
Co-authored-by: Oleksandr Redko <[email protected]>
Co-authored-by: Gourav Kolhatkar <[email protected]>
Co-authored-by: Olivia Bahr <[email protected]>
Co-authored-by: ZipFile <[email protected]>
Co-authored-by: Isak Styf <[email protected]>
Co-authored-by: xiantang <[email protected]>
Co-authored-by: Nemanja Milićević <[email protected]>
Co-authored-by: Brandon Bloom <[email protected]>
Co-authored-by: Nathan Baulch <[email protected]>
Co-authored-by: jingdi.zhu <[email protected]>
Co-authored-by: Polo123456789 <[email protected]>
Co-authored-by: Po <[email protected]>
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

Successfully merging this pull request may close these issues.

Config file not working?
5 participants