Sławomir Grochowski <[email protected]> writes: > Thanks, this clarifies the direction. > > For now, I have kept the current behavior unchanged and pushed the > refactoring as three separate commits: > > org-colview: Extract `org-columns--replace-columns-keyword' > org-colview: Extract `org-columns--insert-columns-keyword' > org-colview: Test COLUMNS keyword storage > > The last commit marks the current insertion behavior explicitly in a > test. I also left the helper comment/FIXME noting that a better > policy may be to insert after the initial block of file-level keywords. > > I will not change the insertion policy in this series. For now I will > continue with the org-colview refactoring and leave this behavioral > change for a follow-up.
So to summarize, all involved have decided that this behavior is bad and should probably be changed. Then you added a test to the test suite that ensures this behavior is present. This situation does remind me of back when I did exactly this 7 months ago and was told to not do that. Here is the relevant link: https://list.orgmode.org/87zf9erj64.fsf@localhost Also see commit e1ef98202 in the org repository. However, switching the test to an ":expected-result :failed" test is not nearly as helpful when you're trying to do some refactoring that shouldn't affect the code behavior. This is exactly why I tried to push back against Ihor in that email thread. Ihor did have some good points though as you can read in that link and commit. Regardless, you should probably remove or ":expected-result :failed" that one test case. Thank you for adding tests! I do love tests! Morgan
