[PATCH] D83759: [clangd] Fixes in lit tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/test/background-index.test:6
+# RUN: sed -e "s|DIRECTORY|%/t|" %/t/definition.jsonrpc > %/t/definition.jsonrpc.1
+# RUN: sed -i.bak -e "s|DIRECTORY|%/t|" %/t/compile_commands.json
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
----------------
ArcsinX wrote:
> kadircet wrote:
> > i don't think there's much point in dropping `-i` from only some commands.
> > 
> > I would suggest either:
> > - dropping it from all, by changing the original files to have a `.tmpl` suffix, and dropping the suffix in sed'd versions
> > - keeping it for all comments by adding a `-i.bak` to all
> For `definition.jsonrpc` we have two `sed` commands and we can keep the file name unchanged without `-i` option.
> But for `compile_commands.json` we have only one `sed` command and we need `-i` option to keep the file name unchanged.
> 
> 
> > changing the original files to have a .tmpl suffix
> In other tests `.1` suffix is used, that's why I use this suffix here.
> E.g. `compile-commands-path-in-initialize.test`
> ```
> # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
> ...
> # RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %t.test.1 > %t.test
> ```
> For definition.jsonrpc we have two sed commands and we can keep the file name unchanged without -i option.

yes for that one we could still move into a `.1` (or `.tmp`) suffixed version.

> In other tests .1 suffix is used, that's why I use this suffix here.

right, I was talking about renaming `clang-tools-extra/clangd/test/Inputs/background-index/definition.jsonrpc` to `clang-tools-extra/clangd/test/Inputs/background-index/definition.jsonrpc.tmpl` (likewise the compile_commands.json). That way you can always do a sed, and write to a different file (i.e. the version without the suffix).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83759/new/

https://reviews.llvm.org/D83759