Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

mac: Fix compilation errors against Node > v5 #56

Closed
wants to merge 2 commits into from
+11 −22
Closed

mac: Fix compilation errors against Node > v5 #56

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2f623a
mac: Fix compilation errors against Node > v5
hhellyer Feb 8, 2017
8c046aa
mac: Fix compilation errors against Node > v5
hhellyer Feb 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion binding.gyp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
'variables': {
'node_major_version': '<!(node -e "console.log(process.version.substr(1,1))")',
Copy link
Member

Choose a reason for hiding this comment

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

We can shorten this line by using -p instead of -e:

       'node_major_version': '<!(node -p "process.version.substr(1,1)")',

},
"targets": [
{
"target_name": "api",
Expand All @@ -13,7 +16,7 @@
"libraries": [ "dbghelp.lib", "Netapi32.lib", "PsApi.lib" ],
"dll_files": [ "dbghelp.dll", "Netapi32.dll", "PsApi.dll" ],
}],
["OS=='mac'", {
["OS=='mac' and node_major_version > 5", {
"xcode_settings": {
'CLANG_CXX_LIBRARY': 'libc++',
'CLANG_CXX_LANGUAGE_STANDARD':'c++11',
Expand Down
27 changes: 11 additions & 16 deletions src/node_report.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,26 +264,21 @@ void SetVersionString(Isolate* isolate) {
// (ares: 1.10.1-DEV, http_parser: 2.7.0, icu: 57.1, modules: 48,
// openssl: 1.0.2j, uv: 1.9.1, v8: 5.1.281.84, zlib: 1.2.8)
const size_t wrap = 80;
size_t line_length = 1;
version_string += "(";
const char* separator = "";
std::string versions = "";
for (auto it : comp_versions) {
std::string component_name = it.first;
std::string component_version = it.second;
size_t length = component_name.length() + component_version.length() + 2;
if (line_length + length + 1 >= wrap) {
version_string += "\n ";
line_length = 1;
}
version_string += component_name;
version_string += ": ";
version_string += component_version;
line_length += length;
if (it != *std::prev(comp_versions.end())) {
version_string += ", ";
line_length += 2;
std::string comp_version_string = it.first;
comp_version_string += ": ";
comp_version_string += it.second;
versions += separator;
if (wrap - (versions.length() % wrap) < comp_version_string.length()) {
Copy link
Member

Choose a reason for hiding this comment

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

The separator is unconditionally appended before the wrapping check, so could take the line over the wrap if the line was right up against the wrap prior to the separator being appended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it matters, the line stays at a sensible length. We don't worry about wrapping elsewhere in the file and plenty of other lines go over 80 characters (OS version and stack trace at least) and we don't wrap those. (Wrapping the stack trace would be horrible.)

Possibly we should just remove the wrapping code for consistency or give each version it's own line.

Copy link
Member

Choose a reason for hiding this comment

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

I had one component/version per line originally in #30 but was talked out of it.

versions += "\n ";
}
separator = ", ";
versions += comp_version_string;
}
version_string += ")\n";
version_string += versions + ")\n";
}

/*******************************************************************************
Expand Down