Skip to content
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

Merge IniAccessUnix.cpp into IniAccess.cpp #159

Merged
merged 6 commits into from
Jul 4, 2022
Merged

Conversation

sksat
Copy link
Collaborator

@sksat sksat commented Jul 4, 2022

Overview

SSIA

Issue

Details

Write in detail.

Validation results

Link to tests or validation results.

Scope of influence

eg. The behavior of XX will be change.

Supplement

Write additional comments if you need.

Note

  • If there are related Projects, tie them together.
  • Assignees should be set if possible.
  • Reviewers should be set if possible.
  • Set priority label if possible.

@sksat sksat self-assigned this Jul 4, 2022
@sksat
Copy link
Collaborator Author

sksat commented Jul 4, 2022

そもそもWin32 APIへの直接の依存を無くしたいような気もしつつ,結局共通しているコードもあるのにファイルが分かれているのは取り回しにくいのでまとめたい,というやつです

@sksat
Copy link
Collaborator Author

sksat commented Jul 4, 2022

これかなりわかんないんだよな

if (path.substr(3) == "ini") {

@sksat
Copy link
Collaborator Author

sksat commented Jul 4, 2022

inihの実装もこういうかんじなんで,単純にこのif文全く要らなさそうだな(そもそも意図が不明だが)

INIReader::INIReader(const string& filename) { _error = ini_parse(filename.c_str(), ValueHandler, this); }
INIReader::INIReader(const char* buffer, size_t buffer_size) {
string content(buffer, buffer_size);
_error = ini_parse_string(content.c_str(), ValueHandler, this);
}
int INIReader::ParseError() const { return _error; }

@sksat
Copy link
Collaborator Author

sksat commented Jul 4, 2022

あーなるほど.このクラス,iniと言いつつCSVも読んでるからか.とはいえsubstr(3)は誤りだな

@sksat
Copy link
Collaborator Author

sksat commented Jul 4, 2022

std::string::ends_with()がC++20でガッカリ

@sksat
Copy link
Collaborator Author

sksat commented Jul 4, 2022

多少修正も入れてしまいましたが,基本的にはIniAccessUnix.cppIniAccess.cppに突っ込んで,差分をifdefしただけです.リファクタリングは別PRでやります.

@sksat sksat requested review from 200km and meltingrabbit July 4, 2022 10:04
@sksat sksat mentioned this pull request Jul 4, 2022
Copy link
Contributor

@meltingrabbit meltingrabbit left a comment

Choose a reason for hiding this comment

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

丁寧にまとめてることを確認した

@sksat sksat merged commit e6f4f44 into develop Jul 4, 2022
@sksat sksat deleted the feature/merge-ini-access branch July 4, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants