Fix the two other clang-tidy warnings

src/DictionaryService.cpp

@@ -118,18 +118,51 @@ void McBopomofo::DictionaryServices::load() {
   FILE* file = fopen(dictionaryServicesPath.c_str(), "r");
   if (!file) {
     FCITX_MCBOPOMOFO_INFO()
-        << "No dictionary service file" << dictionaryServicesPath;
+        << "No dictionary service file: " << dictionaryServicesPath;
     return;
   }
-  fseek(file, 0, SEEK_END);
+  int err = fseek(file, 0, SEEK_END);
+  if (err != 0) {
+    FCITX_MCBOPOMOFO_ERROR()
+        << "Error seeking dictionary service file: " << dictionaryServicesPath;
+    (void)fclose(file);
+    return;
+  }
+
   long file_size = ftell(file);
-  fseek(file, 0, SEEK_SET);
+  if (file_size == -1) {
+    FCITX_MCBOPOMOFO_ERROR()
+        << "Error ftelling dictionary service file: " << dictionaryServicesPath;
+    (void)fclose(file);
+    return;
+  }
+  err = fseek(file, 0, SEEK_SET);
+  if (err != 0) {
+    FCITX_MCBOPOMOFO_ERROR()
+        << "Error seeking dictionary service file: " << dictionaryServicesPath;
+    (void)fclose(file);
+    return;
+  }
+
   std::unique_ptr<char[]> json_data(new char[file_size + 1]);
-  fread(json_data.get(), 1, file_size, file);
-  fclose(file);
+  size_t nchunkread = fread(json_data.get(), file_size, 1, file);
+  if (nchunkread != 1) {
+    FCITX_MCBOPOMOFO_ERROR()
+        << "Error reading dictionary service file: " << dictionaryServicesPath;
+    (void)fclose(file);
+    return;
+  }
+  err = fclose(file);
+  if (err != 0) {
+    FCITX_MCBOPOMOFO_ERROR()
+        << "Error closing dictionary service file: " << dictionaryServicesPath;
+    // Pass through since we've already read the data.
+  }
   json_data[file_size] = '\0';
   struct json_object* json_obj = json_tokener_parse(json_data.get());
   if (json_obj == nullptr) {
+    FCITX_MCBOPOMOFO_WARN()
+        << "Dictionary service file not valid: " << dictionaryServicesPath;
     return;
   }
   struct json_object* servicesArray;

src/InputState.h

@@ -85,7 +85,7 @@ struct Inputting : NotEmpty {
             const std::string_view& tooltipText = "")
       : NotEmpty(buf, index, tooltipText) {}
 
-  Inputting(Inputting const& state)
+  Inputting(const Inputting& state)
       : NotEmpty(state.composingBuffer, state.cursorIndex, state.tooltip) {}
 };
 
@@ -97,7 +97,7 @@ struct ChoosingCandidate : NotEmpty {
                     std::vector<Candidate> cs)
       : NotEmpty(buf, index), candidates(std::move(cs)) {}
 
-  ChoosingCandidate(ChoosingCandidate const& state)
+  ChoosingCandidate(const ChoosingCandidate& state)
       : NotEmpty(state.composingBuffer, state.cursorIndex),
         candidates(state.candidates) {}
 
@@ -138,7 +138,7 @@ struct Marking : NotEmpty {
         reading(std::move(readingText)),
         acceptable(canAccept) {}
 
-  Marking(Marking const& state)
+  Marking(const Marking& state)
       : NotEmpty(state.composingBuffer, state.cursorIndex, state.tooltip),
         markStartGridCursorIndex(state.markStartGridCursorIndex),
         head(state.head),
@@ -166,7 +166,7 @@ struct SelectingDictionary : NotEmpty {
         selectedCandidateIndex(selectedIndex),
         menu(std::move(menu)) {}
 
-  SelectingDictionary(SelectingDictionary const& state)
+  SelectingDictionary(const SelectingDictionary& state)
       : NotEmpty(state.previousState->composingBuffer,
                  state.previousState->cursorIndex,
                  state.previousState->tooltip),

src/Log.h

@@ -28,7 +28,8 @@
 
 FCITX_DECLARE_LOG_CATEGORY(mcbopomofo_log);
 
-#define FCITX_MCBOPOMOFO_WARN() FCITX_LOGC(::mcbopomofo_log, Warn)
+#define FCITX_MCBOPOMOFO_ERROR() FCITX_LOGC(::mcbopomofo_log, Error)
 #define FCITX_MCBOPOMOFO_INFO() FCITX_LOGC(::mcbopomofo_log, Info)
+#define FCITX_MCBOPOMOFO_WARN() FCITX_LOGC(::mcbopomofo_log, Warn)
 
 #endif  // SRC_LOG_H_

src/McBopomofo.cpp

@@ -43,6 +43,17 @@ namespace McBopomofo {
 
 constexpr char kConfigPath[] = "conf/mcbopomofo.conf";
 
+// These two are used to determine whether Shift-[1-9] is pressed.
+constexpr int kFcitxRawKeycode_1 = 10;
+constexpr int kFcitxRawKeycode_9 = 18;
+
+// For determining whether Shift-Enter is pressed in the candidate panel.
+constexpr int kFcitxRawKeycode_Enter = 36;
+
+// If a horizontal panel contains a candidate that's longer than this number,
+// the panel will be changed to a vertical panel.
+constexpr size_t kForceVerticalCandidateThreshold = 8;
+
 static Key MapFcitxKey(const fcitx::Key& key) {
   if (key.isSimple()) {
     return Key::asciiKey(key.sym(), false);
@@ -512,9 +523,9 @@ bool McBopomofoEngine::handleCandidateKeyEvent(
   if (dynamic_cast<InputStates::AssociatedPhrasesPlain*>(state_.get()) !=
       nullptr) {
     int code = origKey.code();
-    if ((origKey.states() & fcitx::KeyState::Shift) && code >= 10 &&
-        code <= 19) {
-      int idx = code - 10;
+    if ((origKey.states() & fcitx::KeyState::Shift) &&
+        code >= kFcitxRawKeycode_1 && code <= kFcitxRawKeycode_9) {
+      int idx = code - kFcitxRawKeycode_1;
       if (idx < candidateList->size()) {
 #ifdef USE_LEGACY_FCITX5_API
         candidateList->candidate(idx)->select(context);
@@ -527,14 +538,12 @@ bool McBopomofoEngine::handleCandidateKeyEvent(
 
   } else {
     int idx = key.keyListIndex(selectionKeys_);
-    if (idx >= 0) {
-      if (idx < candidateList->size()) {
+    if (idx != -1 && idx < candidateList->size()) {
 #ifdef USE_LEGACY_FCITX5_API
-        candidateList->candidate(idx)->select(context);
+      candidateList->candidate(idx)->select(context);
 #else
-        candidateList->candidate(idx).select(context);
+      candidateList->candidate(idx).select(context);
 #endif
-      }
       return true;
     }
   }
@@ -576,7 +585,8 @@ bool McBopomofoEngine::handleCandidateKeyEvent(
   }
 
   if (keyHandler_->inputMode() == McBopomofo::InputMode::McBopomofo &&
-      config_.associatedPhrasesEnabled.value() && origKey.code() == 36 &&
+      config_.associatedPhrasesEnabled.value() &&
+      origKey.code() == kFcitxRawKeycode_Enter &&
       (origKey.states() & fcitx::KeyState::Shift)) {
     int idx = candidateList->cursorIndex();
     if (idx < candidateList->size()) {
@@ -899,6 +909,8 @@ void McBopomofoEngine::handleCandidatesState(fcitx::InputContext* context,
 
   if (dynamic_cast<InputStates::AssociatedPhrasesPlain*>(state_.get()) !=
       nullptr) {  // NOLINT(bugprone-branch-clone)
+    // Associated phrases in Plain Bopomofo only takes Shift-[1-9]; we push
+    // these keys and will detect the shift mask later.
     selectionKeys_.emplace_back(FcitxKey_1);
     selectionKeys_.emplace_back(FcitxKey_2);
     selectionKeys_.emplace_back(FcitxKey_3);
@@ -1116,7 +1128,8 @@ fcitx::CandidateLayoutHint McBopomofoEngine::getCandidateLayoutHint() const {
     for (const InputStates::ChoosingCandidate::Candidate& candidate :
          candidates) {
       std::string value = candidate.value;
-      if (McBopomofo::CodePointCount(value) > 8) {
+      if (McBopomofo::CodePointCount(value) >
+          kForceVerticalCandidateThreshold) {
         return fcitx::CandidateLayoutHint::Vertical;
       }
     }