ArcsinX marked an inline comment as done. ArcsinX 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:/..." ---------------- kadircet wrote: > 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). Got it, fixed, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83759/new/ https://reviews.llvm.org/D83759 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits