Skip to content

DLL-inteface for operator<< (MSVC compatibility) #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ set(docopt_HEADERS
docopt_private.h
docopt_util.h
docopt_value.h
docopt_api.h
)

#============================================================================
Expand Down
19 changes: 11 additions & 8 deletions docopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
#include <cassert>
#include <cstddef>

using namespace docopt;
namespace docopt {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why you're doing this? I don't necessarily oppose it but would like to understand the justification. (Especially because you labeled this as something to do with GCC, which shouldn't be an issue).

One reason I like the "docopt::" in the definitions is that it catches bugs (if a function gets defined that didn't have a declaration).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, use previous style


DOCOPT_INLINE
std::ostream& docopt::operator<<(std::ostream& os, value const& val)
std::ostream& operator<<(std::ostream& os, value const& val)
{
if (val.isBool()) {
bool b = val.asBool();
Expand Down Expand Up @@ -610,7 +610,7 @@ static std::pair<Required, std::vector<Option>> create_pattern_tree(std::string

DOCOPT_INLINE
std::map<std::string, value>
docopt::docopt_parse(std::string const& doc,
docopt_parse(std::string const& doc,
std::vector<std::string> const& argv,
bool help,
bool version,
Expand Down Expand Up @@ -660,11 +660,11 @@ docopt::docopt_parse(std::string const& doc,

DOCOPT_INLINE
std::map<std::string, value>
docopt::docopt(std::string const& doc,
std::vector<std::string> const& argv,
bool help,
std::string const& version,
bool options_first) noexcept
docopt(std::string const& doc,
std::vector<std::string> const& argv,
bool help,
std::string const& version,
bool options_first) noexcept
{
try {
return docopt_parse(doc, argv, help, !version.empty(), options_first);
Expand All @@ -685,3 +685,6 @@ docopt::docopt(std::string const& doc,
std::exit(-1);
} /* Any other exception is unexpected: let std::terminate grab it */
}

} /* namespace docopt */

32 changes: 5 additions & 27 deletions docopt.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,37 +9,13 @@
#ifndef docopt__docopt_h_
#define docopt__docopt_h_

#include "docopt_api.h"
#include "docopt_value.h"

#include <map>
#include <vector>
#include <string>

#ifdef DOCOPT_HEADER_ONLY
#define DOCOPT_INLINE inline
#define DOCOPT_API
#else
#define DOCOPT_INLINE

// With Microsoft Visual Studio, export certain symbols so they
// are available to users of docopt.dll (shared library). The DOCOPT_DLL
// macro should be defined if building a DLL (with Visual Studio),
// and by clients using the DLL. The CMakeLists.txt and the
// docopt-config.cmake it generates handle this.
#ifdef DOCOPT_DLL
// Whoever is *building* the DLL should define DOCOPT_EXPORTS.
// The CMakeLists.txt that comes with docopt does this.
// Clients of docopt.dll should NOT define DOCOPT_EXPORTS.
#ifdef DOCOPT_EXPORTS
#define DOCOPT_API __declspec(dllexport)
#else
#define DOCOPT_API __declspec(dllimport)
#endif
#else
#define DOCOPT_API
#endif
#endif

namespace docopt {

// Usage string could not be parsed (ie, the developer did something wrong)
Expand Down Expand Up @@ -67,7 +43,8 @@ namespace docopt {
/// @throws DocoptExitHelp if 'help' is true and the user has passed the '--help' argument
/// @throws DocoptExitVersion if 'version' is true and the user has passed the '--version' argument
/// @throws DocoptArgumentError if the user's argv did not match the usage patterns
std::map<std::string, value> DOCOPT_API docopt_parse(std::string const& doc,
DOCOPT_API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little nervous about swapping the order of these (the return type and the annotation). Was this intentional?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__declspec(dllexport)/__declspec(dllimport) should be of left-side */& (MSVC specfic). This change for one-style with operator<<

std::map<std::string, value> docopt_parse(std::string const& doc,
std::vector<std::string> const& argv,
bool help = true,
bool version = true,
Expand All @@ -80,7 +57,8 @@ namespace docopt {
/// * DocoptExitHelp - print usage string and terminate (with exit code 0)
/// * DocoptExitVersion - print version and terminate (with exit code 0)
/// * DocoptArgumentError - print error and usage string and terminate (with exit code -1)
std::map<std::string, value> DOCOPT_API docopt(std::string const& doc,
DOCOPT_API
std::map<std::string, value> docopt(std::string const& doc,
std::vector<std::string> const& argv,
bool help = true,
std::string const& version = {},
Expand Down
37 changes: 37 additions & 0 deletions docopt_api.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//
// docopt.h
// docopt
//
// Created by Dmitriy Vetutnev on 2019-09-05.
// Copyright (c) 2019 Dmitriy Vetutnev. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the attribution and copyright here? I don't think we necessarily need any copyright attribution outside of the top one. (In other words, I'm not asking you to change it match the docopt.h header, just leave only the lines 1-4 and delete 5 and 6).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this lines removed

//

#ifndef docopt__api_h_
#define docopt__api_h_

#ifdef DOCOPT_HEADER_ONLY
#define DOCOPT_INLINE inline
#define DOCOPT_API
#else
#define DOCOPT_INLINE

// With Microsoft Visual Studio, export certain symbols so they
// are available to users of docopt.dll (shared library). The DOCOPT_DLL
// macro should be defined if building a DLL (with Visual Studio),
// and by clients using the DLL. The CMakeLists.txt and the
// docopt-config.cmake it generates handle this.
#ifdef DOCOPT_DLL
// Whoever is *building* the DLL should define DOCOPT_EXPORTS.
// The CMakeLists.txt that comes with docopt does this.
// Clients of docopt.dll should NOT define DOCOPT_EXPORTS.
#ifdef DOCOPT_EXPORTS
#define DOCOPT_API __declspec(dllexport)
#else
#define DOCOPT_API __declspec(dllimport)
#endif
#else
#define DOCOPT_API
#endif
#endif

#endif /* defined(docopt__api_h_) */
4 changes: 3 additions & 1 deletion docopt_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#ifndef docopt__value_h_
#define docopt__value_h_

#include "docopt_api.h"

#include <stdexcept>
#include <string>
#include <vector>
Expand Down Expand Up @@ -105,7 +107,7 @@ namespace docopt {
};

/// Write out the contents to the ostream
std::ostream& operator<<(std::ostream&, value const&);
DOCOPT_API std::ostream& operator<<(std::ostream&, const value&);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change back to value const& (the style for this library is east-const)

Copy link
Author

@dvetutnev dvetutnev Sep 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, changed

}

namespace std {
Expand Down