https://pantsbuild.org/ logo
Powered by
f

future-oxygen-10553

05/04/2023, 10:20 PM
Another debugging question… I’m trying to include a config file for a formatter in the tests (here). However, the output from the formatter shows that the config file was not picked up. Do I need to do something extra to put the config file into the sandbox?
Based on what 
shfmt
does, it doesn’t seem so, but then again this is now a targetless formtter which may be different
b

bitter-ability-32190

05/04/2023, 10:26 PM
In your normal rule, doing the config discovery object and pike around and make sure it looks right. Also see if the digest for the configs has bytes
f

future-oxygen-10553

05/04/2023, 10:27 PM
It does, that 
print(var[0].content)
you commented on shows the correct output 😉 But the output from taplo should show that it ignored one file and it only shows the two files it’s supposed to format. Example in the next message…
Copy code
tree -a taplo-tests/
taplo-tests/
├── .taplo.toml
└── tester.toml
Copy code
taplo fmt taplo-tests/
 INFO taplo:format_files:collect_files: found files total=1 excluded=1 cwd="/Users/bweber/Documents"
So it found the 
tester.toml
and the 
.taplo.toml
and excluded the latter
But my test output only shows:
Copy code
INFO taplo:format_files:collect_files: found files total=2 excluded=0 cwd="/private/tmp/pants-sandbox-RO8siG"
b

bitter-ability-32190

05/04/2023, 11:09 PM
So just to be clear, the rule is correctly seeing the config?
Does taplo require the config at CWD? Or does it allow configs up directory chain from (somewhere)
(I'll try and clone your branch today, and poke around)
f

future-oxygen-10553

05/05/2023, 2:27 PM
The rule is able to print out the config, but Taplo does not see it. Yes, Taplo only looks in the cwd for configs
b

bitter-ability-32190

05/05/2023, 2:28 PM
That might be a clue
f

future-oxygen-10553

05/05/2023, 2:29 PM
So it will find the config file and ignore it if it’s not in the cwd but is in the directory specified to format
That’s what I was trying to show with my 
taplo-tests
folder. My cwd is the parent of 
taplo-tests
, so taplo doesn’t use the config file but it does exclude it from formatting
OTOH the test doesn’t even find/ignore the config file
b

bitter-ability-32190

05/05/2023, 6:07 PM
but it does exclude it from formatting
So I see that the strings before/after for the config are different:
Copy code
"a/.taplo.toml": "[formatting]\ncompact_entries = true\n",
"a/.taplo.toml": "[formatting]\ncompact_entries=true\n",
Changing that to match, and, since you said it doesn't find a config, changing the files to still be 
NEEDS_CONFIG_FILE
the test gets past this assert
We have this issue in other plugins as well. We currently only assume one (root) config if the 
--config
option isn't specified
This passes:
Copy code
def test_config_files(rule_runner: RuleRunner) -> None:
    rule_runner.write_files(
        {
            "a/f.toml": NEEDS_CONFIG_FILE,
            ".taplo.toml": "[formatting]\ncompact_entries = true\n",
            "b/f.toml": NEEDS_CONFIG_FILE,
        }
    )
    fmt_result = run_taplo(rule_runner)
    assert fmt_result.stdout == ""
    assert fmt_result.output == rule_runner.make_snapshot(
        {
            "a/f.toml": FIXED_NEEDS_CONFIG_FILE,
            "b/f.toml": FIXED_NEEDS_CONFIG_FILE,
            ".taplo.toml": "[formatting]\ncompact_entries=true\n",
        }
    )
    assert fmt_result.did_change is True
f

future-oxygen-10553

05/05/2023, 8:54 PM
Thanks @bitter-ability-32190! I’ll take a look on Monday 🙂
@bitter-ability-32190 I updated the PR whenever you have a chance to take a look
@bitter-ability-32190 Sorry for the delay, updated the PR again
b

bitter-ability-32190

05/11/2023, 2:46 PM
No worries, life is busy. I am too 😛
f

future-oxygen-10553

05/11/2023, 2:46 PM
❤️
b

bitter-ability-32190

05/11/2023, 2:49 PM
FWIW I keep a pretty close finger on my GitHub notifications. Doesn't mean I get to them quickly but I DO watch
f

future-oxygen-10553

05/11/2023, 3:02 PM
👍
Powered by