From ce35626897d4864b5b4b04e086a0a45500f4e6ab Mon Sep 17 00:00:00 2001
From: anutosh491 <andersonbhat491@gmail.com>
Date: Mon, 23 Dec 2024 01:33:53 +0530
Subject: [PATCH] Address reviews

---
 CMakeLists.txt                         | 12 +++++++-----
 include/xeus-cpp/xinterpreter_wasm.hpp |  3 ---
 src/xinterpreter.cpp                   | 15 ++++++---------
 src/xinterpreter_wasm.cpp              |  8 +-------
 4 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 061a373a..a9a08845 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -152,10 +152,7 @@ function(configure_kernel kernel)
 endfunction()
 
 message("Configure kernels: ...")
-if(NOT EMSCRIPTEN)
-    configure_kernel("/share/jupyter/kernels/xcpp17/")
-    configure_kernel("/share/jupyter/kernels/xcpp20/")
-else()
+if(EMSCRIPTEN)
     # TODO: Currently jupyterlite-xeus and xeus-lite do not provide
     # methods to fetch information from the arguments present in the
     # generated emscripten kernel.
@@ -167,6 +164,9 @@ else()
     # 3) Finally we should fetch the C++ version from the kernel.json file and
     #    be able to pass it to our wasm interpreter rather than forcing a version.
     configure_kernel("/share/jupyter/kernels/xcpp20/")
+else()
+    configure_kernel("/share/jupyter/kernels/xcpp17/")
+    configure_kernel("/share/jupyter/kernels/xcpp20/")
 endif()
 
 # Source files
@@ -415,6 +415,8 @@ if(EMSCRIPTEN)
     xeus_cpp_set_kernel_options(xcpp)
     xeus_wasm_compile_options(xcpp)
     xeus_wasm_link_options(xcpp "web,worker")
+    # TODO: Remove the exported runtime methods
+    # after the next xeus release.
     target_link_options(xcpp PUBLIC
         -sEXPORTED_RUNTIME_METHODS=FS,PATH,ERRNO_CODES
         # add sysroot location here
@@ -423,7 +425,7 @@ if(EMSCRIPTEN)
     # TODO: Emscripten supports preloading files just once before it generates
     # the xcpp.data file (containing the binary representation of the file(s) we
     # want to include in our application).
-    # Hence although we are adding supporting for Standard Headers, Libraries etc
+    # Hence although we are adding support for Standard Headers, Libraries etc
     # through emscripten's sysroot for now, we need to do the following:
     # 1) Enable CppInterOp to provide us with a resource dir.
     # 2) If the above cannot be done, we can use the resource dir provided
diff --git a/include/xeus-cpp/xinterpreter_wasm.hpp b/include/xeus-cpp/xinterpreter_wasm.hpp
index 2c45b1a1..abb7614e 100644
--- a/include/xeus-cpp/xinterpreter_wasm.hpp
+++ b/include/xeus-cpp/xinterpreter_wasm.hpp
@@ -22,9 +22,6 @@ namespace xcpp
         wasm_interpreter();
         virtual ~wasm_interpreter() = default;
 
-    private:
-
-        static std::vector<const char*> create_args();
     };
 }
 
diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp
index 1891d551..845f0410 100644
--- a/src/xinterpreter.cpp
+++ b/src/xinterpreter.cpp
@@ -17,10 +17,8 @@
 
 #include "xinput.hpp"
 #include "xinspect.hpp"
-#ifndef EMSCRIPTEN
 #include "xmagics/os.hpp"
 #include "xmagics/xassist.hpp"
-#endif
 #include "xparser.hpp"
 #include "xsystem.hpp"
 
@@ -28,7 +26,9 @@ using Args = std::vector<const char*>;
 
 void* createInterpreter(const Args &ExtraArgs = {}) {
   Args ClangArgs = {/*"-xc++"*/"-v"}; // ? {"-Xclang", "-emit-llvm-only", "-Xclang", "-diagnostic-log-file", "-Xclang", "-", "-xc++"};
-#ifndef EMSCRIPTEN
+#ifdef EMSCRIPTEN
+  ClangArgs.push_back("-std=c++20");
+#else
   if (std::find_if(ExtraArgs.begin(), ExtraArgs.end(), [](const std::string& s) {
     return s == "-resource-dir";}) == ExtraArgs.end()) {
     std::string resource_dir = Cpp::DetectResourceDir();
@@ -99,8 +99,7 @@ __get_cxx_version ()
         auto cxx_version = Cpp::Evaluate(code);
         return std::to_string(cxx_version);
 #else
-        constexpr int cxx_version = 20;
-        return std::to_string(cxx_version);
+        return "20";
 #endif
     }
 
@@ -115,9 +114,11 @@ __get_cxx_version ()
         createInterpreter(Args(argv ? argv + 1 : argv, argv + argc));
         m_version = get_stdopt();
         redirect_output();
+#ifndef EMSCRIPTEN
         init_includes();
         init_preamble();
         init_magic();
+#endif
     }
 
     interpreter::~interpreter()
@@ -362,9 +363,7 @@ __get_cxx_version ()
 
     void interpreter::init_includes()
     {
-#ifndef EMSCRIPTEN
         Cpp::AddIncludePath((xeus::prefix_path() + "/include/").c_str());
-#endif
     }
 
     void interpreter::init_preamble()
@@ -383,9 +382,7 @@ __get_cxx_version ()
         // preamble_manager["magics"].get_cast<xmagics_manager>().register_magic("timeit",
         // timeit(&m_interpreter));
         // preamble_manager["magics"].get_cast<xmagics_manager>().register_magic("python", pythonexec());
-#ifndef EMSCRIPTEN
         preamble_manager["magics"].get_cast<xmagics_manager>().register_magic("xassist", xassist());
         preamble_manager["magics"].get_cast<xmagics_manager>().register_magic("file", writefile());
-#endif
     }
 }
diff --git a/src/xinterpreter_wasm.cpp b/src/xinterpreter_wasm.cpp
index c3980773..7746c7b6 100644
--- a/src/xinterpreter_wasm.cpp
+++ b/src/xinterpreter_wasm.cpp
@@ -17,13 +17,7 @@ namespace xcpp
 {
 
     wasm_interpreter::wasm_interpreter()
-        : interpreter(create_args().size(), create_args().data())
+        : interpreter(0, nullptr)
     {
     }
-
-    std::vector<const char*> wasm_interpreter::create_args()
-    {
-        static std::vector<const char*> Args = {"-Xclang", "-std=c++20"};
-        return Args;
-    }
 }