Skip to content

Commit 45ad981

Browse files
committed
Address Copilot review feedback for PR 1
dotenv.h: - Clarify inline variable syntax with explanatory comment - Add comment explaining why _instance cannot be inline inside class dotenv.cpp: - Rename Dotenv_Type to DotenvClass for better C++ conventions - Fix operator[] to use typedef alias consistently (avoid parsing issues) - Improve comment clarity for the global env variable definition CMakeLists.txt: - Add MSVC support for compiler flags (/W4 /Od /O2) - Improve Windows library naming: use distinct OUTPUT_NAME on MSVC to avoid conflicts between static and shared library .lib files common/libs/antlr4-cpp-runtime/CMakeLists.txt: - Update cmake_minimum_required from 3.10 to 3.16 for consistency
1 parent 4d1fbbf commit 45ad981

File tree

4 files changed

+34
-19
lines changed

4 files changed

+34
-19
lines changed

CMakeLists.txt

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,21 @@ set(CPP_DOTENV_SRC
4242
# Common include directories
4343
set(CPP_DOTENV_INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/include)
4444

45-
# Common compile options
46-
if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
47-
set(CPP_DOTENV_COMPILE_OPTIONS -g -Wall -O0)
45+
# Common compile options - handle MSVC vs GCC/Clang
46+
if (MSVC)
47+
# MSVC-specific flags
48+
if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
49+
set(CPP_DOTENV_COMPILE_OPTIONS /W4 /Od)
50+
else()
51+
set(CPP_DOTENV_COMPILE_OPTIONS /W4 /O2)
52+
endif()
4853
else()
49-
set(CPP_DOTENV_COMPILE_OPTIONS -O3)
54+
# GCC/Clang and other compilers using Unix-style flags
55+
if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
56+
set(CPP_DOTENV_COMPILE_OPTIONS -g -Wall -O0)
57+
else()
58+
set(CPP_DOTENV_COMPILE_OPTIONS -O3)
59+
endif()
5060
endif()
5161

5262
# Build both static and shared libraries if requested
@@ -70,12 +80,14 @@ if(BUILD_BOTH_LIBRARIES)
7080
)
7181
target_include_directories(cpp_dotenv_shared PUBLIC ${CPP_DOTENV_INCLUDE_DIRS})
7282
target_compile_options(cpp_dotenv_shared PRIVATE ${CPP_DOTENV_COMPILE_OPTIONS})
73-
# Set output name to libcpp_dotenv.so/.dylib (instead of libcpp_dotenv_shared.so)
74-
set_target_properties(cpp_dotenv_shared PROPERTIES OUTPUT_NAME cpp_dotenv)
75-
# On Windows (MSVC), the import library would conflict with the static library.
76-
# Add a suffix to the import library name to avoid this conflict.
83+
# Set output name. On non-MSVC platforms, both static and shared libraries use
84+
# the same base name "cpp_dotenv". On MSVC, give the shared library a distinct
85+
# base name to avoid conflicts between the static library (.lib) and the
86+
# import library for the shared library (also .lib).
7787
if(MSVC)
78-
set_target_properties(cpp_dotenv_shared PROPERTIES IMPORT_SUFFIX "_shared.lib")
88+
set_target_properties(cpp_dotenv_shared PROPERTIES OUTPUT_NAME cpp_dotenv_shared)
89+
else()
90+
set_target_properties(cpp_dotenv_shared PROPERTIES OUTPUT_NAME cpp_dotenv)
7991
endif()
8092

8193
# Alias for backward compatibility:

common/libs/antlr4-cpp-runtime/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
cmake_minimum_required(VERSION 3.10)
1+
cmake_minimum_required(VERSION 3.16)
22
project(antlr4-cpp-runtime VERSION 4.8)
33

44
set(CMAKE_CXX_STANDARD 17)

include/dotenv.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,18 @@ namespace dotenv
3535

3636
private:
3737

38+
// Declare static members (definitions are inline after the class)
3839
static const std::string env_filename;
3940
static dotenv _instance;
4041

4142
};
4243

4344

44-
extern dotenv& env;
45-
46-
// Inline variables defined after class is complete (C++17)
45+
// C++17 inline variable definitions after class is complete
46+
// Note: _instance cannot be inline inside the class due to incomplete type
4747
inline const std::string dotenv::env_filename = ".env";
4848
inline dotenv dotenv::_instance;
49+
50+
51+
extern dotenv& env;
4952
}

src/dotenv.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@
1010
using namespace std;
1111

1212

13-
// Typedef to work around the fact that class name matches namespace name
13+
// Type alias to work around the fact that class name matches namespace name
1414
// This is a known C++ limitation when defining out-of-line members
15-
typedef class ::dotenv::dotenv Dotenv_Type;
15+
typedef class ::dotenv::dotenv DotenvClass;
1616

1717

18-
Dotenv_Type& ::dotenv::dotenv::load_dotenv(const string& dotenv_path, const bool overwrite, const bool interpolate)
18+
DotenvClass& ::dotenv::dotenv::load_dotenv(const string& dotenv_path, const bool overwrite, const bool interpolate)
1919
{
2020
ifstream env_file;
2121
env_file.open(dotenv_path);
@@ -31,17 +31,17 @@ Dotenv_Type& ::dotenv::dotenv::load_dotenv(const string& dotenv_path, const bool
3131
}
3232

3333

34-
auto ::dotenv::dotenv::operator[](const key_type& k) const -> const string
34+
const string DotenvClass::operator[](const key_type& k) const
3535
{
3636
return getenv(k).second;
3737
}
3838

3939

40-
Dotenv_Type& ::dotenv::dotenv::instance()
40+
DotenvClass& ::dotenv::dotenv::instance()
4141
{
4242
return _instance;
4343
}
4444

4545

46-
// Static members are now inline in the header (C++17)
46+
// Define the global 'env' reference. Static members are inline in the header (C++17).
4747
::dotenv::dotenv& ::dotenv::env = ::dotenv::dotenv::instance();

0 commit comments

Comments
 (0)