Added environment variable resolution to the include directive in a config file #200

billh-apcon-com · 2016-10-06T13:15:11Z

Added environment variable resolution to the include directive in a config file

…onfig file

wilx · 2016-10-06T20:50:13Z

msvc14/Qt5DebugAppender.vcxproj

  <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug_Unicode|Win32'">
    <Link>
      <AdditionalDependencies>Qt5Cored.lib;%(AdditionalDependencies)</AdditionalDependencies>
+      <AdditionalLibraryDirectories>C:\Qt\5.6.0\lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories>


Do not add the changes to the Visual Studio project files to the pull request.

wilx · 2016-10-06T20:50:30Z

include/log4cplus/helpers/property.h


          // Methods
            void init(log4cplus::tistream& input);
+			bool substitueEnvVariables(tstring &fileName, tstring &modFileName);


Please remote all tabs from the source.

wilx · 2016-10-06T20:50:58Z

msvc14/log4cplus.sln

 		{7EFABA06-71CD-498F-BF10-C41A7D2DCF3B} = {92DEA81D-81ED-4283-BBC7-41975647F69E}
 		{AE4CF05D-9770-4BF2-BB73-1DA37E2A1508} = {92DEA81D-81ED-4283-BBC7-41975647F69E}
 		{C0C6F651-99D8-404E-B2EC-507B261E820C} = {DAFB5F7D-6AF3-4D4F-B6D3-C1049A1259DA}
-		{18B64AA1-A2F7-46BE-8D48-7882AA471FA9} = {DAFB5F7D-6AF3-4D4F-B6D3-C1049A1259DA}


Do not add the changes to the Visual Studio project files to the pull request.

wilx · 2016-10-06T20:51:26Z

src/property.cxx

-        {
-            tstring included (buffer, 8) ;
-            trim_ws (included);
+		else if (buffer.compare(0, 7, LOG4CPLUS_TEXT("include")) == 0


Remove tabs.

wilx · 2016-10-06T20:54:42Z

src/property.cxx

 }

+bool
+Properties::substitueEnvVariables(tstring &fileName, tstring &modFileName)


I am not sure that we need a new function for this. There is already a function substVars() in configurator.cxx. It seems it should be used here as well. If so, you can probably move it from anonymous namespace into internal namespace and make it visible to the property.cxx and use it there.

billh-apcon-com · 2016-10-07T12:37:08Z

Sorry about the changes in the visual studio projects. I’m new to Git, and it’s a little different from other CMS packages.

I’ll look into your suggestion about substVars()

log4cplus/src/configurator.cxx

Line 102 in 078ee67

substVars (tstring & dest, const tstring & val,

.

For now, just reject the pull request.

Regards,
William Hayden
Apcon, Inc

From: Václav Haisman [mailto:[email protected]]
Sent: Thursday, October 06, 2016 3:55 PM
To: log4cplus/log4cplus [email protected]
Cc: Bill Hayden [email protected]; Author [email protected]
Subject: Re: [log4cplus/log4cplus] Added environment variable resolution to the include directive in a config file (#200)

@wilx requested changes on this pull request.

In msvc14/Qt5DebugAppender.vcxprojhttps://github.com//pull/200#pullrequestreview-3201271:

@@ -138,7 +138,11 @@

 <Link>

   <AdditionalDependencies>Qt5Cored.lib;%(AdditionalDependencies)</AdditionalDependencies>

 <AdditionalLibraryDirectories>C:\Qt\5.6.0\lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories>

Do not add the changes to the Visual Studio project files to the pull request.

In include/log4cplus/helpers/property.hhttps://github.com//pull/200#pullrequestreview-3201271:

@@ -140,6 +140,7 @@ namespace log4cplus {

       // Methods

         void init(log4cplus::tistream& input);

                 bool substitueEnvVariables(tstring &fileName, tstring &modFileName);

Please remote all tabs from the source.

In msvc14/log4cplus.slnhttps://github.com//pull/200#pullrequestreview-3201271:

@@ -786,7 +768,6 @@ Global

           {7EFABA06-71CD-498F-BF10-C41A7D2DCF3B} = {92DEA81D-81ED-4283-BBC7-41975647F69E}

           {AE4CF05D-9770-4BF2-BB73-1DA37E2A1508} = {92DEA81D-81ED-4283-BBC7-41975647F69E}

           {C0C6F651-99D8-404E-B2EC-507B261E820C} = {DAFB5F7D-6AF3-4D4F-B6D3-C1049A1259DA}

         {18B64AA1-A2F7-46BE-8D48-7882AA471FA9} = {DAFB5F7D-6AF3-4D4F-B6D3-C1049A1259DA}

Do not add the changes to the Visual Studio project files to the pull request.

In src/property.cxxhttps://github.com//pull/200#pullrequestreview-3201271:

@@ -256,27 +256,53 @@ Properties::init(tistream& input)

         trim_ws (value);

         setProperty(key, value);

     }

   else if (buffer.compare (0, 7, LOG4CPLUS_TEXT ("include")) == 0

```
       && buffer.size () >= 7 + 1 + 1
```
```
       && is_space (buffer[7]))
```
```
   {
```
```
       tstring included (buffer, 8) ;
```
```
       trim_ws (included);
```

         else if (buffer.compare(0, 7, LOG4CPLUS_TEXT("include")) == 0

Remove tabs.

In src/property.cxxhttps://github.com//pull/200#pullrequestreview-3201271:

         init (file);

     }

 }

}

+bool

+Properties::substitueEnvVariables(tstring &fileName, tstring &modFileName)

I am not sure that we need a new function for this. There is already a function substVars()

log4cplus/src/configurator.cxx

Line 102 in 078ee67

substVars (tstring & dest, const tstring & val,

in configurator.cxx. It seems it should be used here as well. If so, you can probably move it from anonymous namespace into internal namespace and make it visible to the property.cxx and use it there.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//pull/200#pullrequestreview-3201271, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AVmKVf4EqvutvGeDG3KU1CmMe7_ml-T_ks5qxWArgaJpZM4KP7P4.

billh-apcon-com added 2 commits October 5, 2016 15:37

Added environment variable resolution to the include directive in a c…

18c9f67

…onfig file

Added environment variable resolution to the include directive in a c…

078ee67

…onfig file

wilx requested changes Oct 6, 2016

View reviewed changes

wilx self-assigned this Oct 6, 2016

wilx added the enhancement label Oct 6, 2016

murphi mentioned this pull request Sep 2, 2023

config: resolve environment variables in include #592

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added environment variable resolution to the include directive in a config file #200

Added environment variable resolution to the include directive in a config file #200

Uh oh!

billh-apcon-com commented Oct 6, 2016

Uh oh!

wilx Oct 6, 2016

Uh oh!

wilx Oct 6, 2016

Uh oh!

wilx Oct 6, 2016

Uh oh!

wilx Oct 6, 2016 •

edited

Loading

Uh oh!

wilx Oct 6, 2016

Uh oh!

billh-apcon-com commented Oct 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Added environment variable resolution to the include directive in a config file #200

Are you sure you want to change the base?

Added environment variable resolution to the include directive in a config file #200

Uh oh!

Conversation

billh-apcon-com commented Oct 6, 2016

Uh oh!

wilx Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

wilx Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

wilx Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

wilx Oct 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilx Oct 6, 2016

Choose a reason for hiding this comment

Uh oh!

billh-apcon-com commented Oct 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wilx Oct 6, 2016 •

edited

Loading