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

Added support for the Meson Build System #298

Merged
merged 6 commits into from
Nov 23, 2016
Merged

Added support for the Meson Build System #298

merged 6 commits into from
Nov 23, 2016

Conversation

eidheim
Copy link
Member

@eidheim eidheim commented Nov 12, 2016

Should be 100%, but some additional testing needed. Let me know if there are any issues or not.

@eidheim eidheim mentioned this pull request Nov 12, 2016
@eidheim eidheim force-pushed the meson branch 5 times, most recently from 7638e5a to 3864fc9 Compare November 13, 2016 19:19
@zalox
Copy link
Member

zalox commented Nov 14, 2016

I'll have a look, please give me some time to learn meson.

@eidheim
Copy link
Member Author

eidheim commented Nov 14, 2016

Sure, and thank you:)

else if(parameter==paramter_name)
found_argument=true;
}

Copy link
Member

Choose a reason for hiding this comment

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

std::copy_if(parameters.begin(),paramerters.end(),parameter_values.end(),[](){}));

Copy link
Member Author

Choose a reason for hiding this comment

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

The function gets more complicated if copy_if is to be used:

std::vector<std::string> CompileCommands::Command::paramter_values(const std::string &parameter_name) const {
  std::vector<std::string> parameter_values(parameters);

  bool found_argument=false;
  auto it=std::copy_if(parameters.begin(), parameters.end(), parameter_values.begin(),
               [&parameter_name, &found_argument](const std::string &parameter){
    if(found_argument) {
      found_argument=false;
      return true;
    }
    else if(parameter==parameter_name) {
      found_argument=true;
      return false;
    }
    return false;
  });
  parameter_values.resize(std::distance(parameter_values.begin(), it));

  return parameter_values;
}

Copy link
Member

@zalox zalox Nov 15, 2016

Choose a reason for hiding this comment

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

std::copy_if(parameters.begin(),paramerters.end(),parameter_values.end(),[&parameter_name](const std::string &param){
  return parameter_name == param;
}));

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not the same. The idea is to return for instance some_file after -o some_file when the parameter_name is -o.

Copy link
Member

Choose a reason for hiding this comment

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

Ohh, sorry I missed the else if, but at least this made me notice the miss spelt paramter_name.

Copy link
Member Author

Choose a reason for hiding this comment

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

I fixed the misspelling yesterday:)

Copy link
Member

Choose a reason for hiding this comment

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

Did you push the spellfix? It still says paramter for me.

try {
boost::property_tree::ptree root_pt;
boost::property_tree::json_parser::read_json((build_path/"compile_commands.json").string(), root_pt);

Copy link
Member

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but I think read_json can cause problems. I think there is a error_code version.

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be safe, since the exceptions are caught.

backslash=true;
else if((parameters_str[c]==' ' || parameters_str[c]=='\t') && !backslash && !single_quote && !double_quote) {
if(parameter_start_pos!=static_cast<size_t>(-1)) {
add_parameter();
Copy link
Member

Choose a reason for hiding this comment

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

std::npos?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, will fix

return false;
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggestion:

if(project_path.empty() || default_build_path.empty())
    return false;
if(!boost::filesystem::exists(project_path/"meson.build"))
   return false;

Copy link
Member Author

Choose a reason for hiding this comment

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

Will clean up this as well.

return false;
}
}

Copy link
Member

Choose a reason for hiding this comment

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

This calls for abstraction, but I agree to wait if we are introducing more build systems.

@eidheim
Copy link
Member Author

eidheim commented Nov 20, 2016

@zalox Are you happy with the changes? Can we merge this PR?

@eidheim
Copy link
Member Author

eidheim commented Nov 21, 2016

@zalox the spelling errors should be fixed now. Thought that I had fixed them, but apparently not all.

@zalox
Copy link
Member

zalox commented Nov 23, 2016

Looks good, sorry about my late responses, been flying all over Europe!

@zalox zalox merged commit cf530b4 into cppit:master Nov 23, 2016
@eidheim eidheim deleted the meson branch November 23, 2016 12:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants