atilaneves / cmake-ide

Use Emacs as a C/C++ IDE
BSD 3-Clause "New" or "Revised" License
716 stars 92 forks source link

cmake-ide-cmake-opts can't contain parameters with spaces #181

Closed andrmuel closed 5 years ago

andrmuel commented 5 years ago

Due to the 'split-string' in cide--run-cmake-impl, it is not possible to have a space in a parameter in cmake-ide-cmake-opts (e.g. "-DCONF_FILES=foo.conf bar.conf").

My workaround for the moment is to allow passing the parameters as lists:

(defun cide--run-cmake-impl (project-dir cmake-dir)
  "Run the CMake process for PROJECT-DIR in CMAKE-DIR."
  (when project-dir
    (let ((default-directory cmake-dir))
      (cide--message "Running cmake for src path %s in build path %s" project-dir cmake-dir)
      (apply 'start-process (append (list "cmake" "*cmake*" cmake-ide-cmake-command)
                                    (if (listp cmake-ide-cmake-opts) cmake-ide-cmake-opts (split-string cmake-ide-cmake-opts))
                                    (list "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON" project-dir))))))

(defun cide--project-key ()
  "Return a unique key for a project based on the project dir and cmake options."
  (let ((project-dir (cide--locate-project-dir)))
    (when project-dir
      ;; if no project-dir, then get-project-key is called from a non cmake project dir, simply ignore
      (replace-regexp-in-string "[-/= ]" "_"  (concat (expand-file-name project-dir)
                                                      (if (listp cmake-ide-cmake-opts) (string-join cmake-ide-cmake-opts " ") cmake-ide-cmake-opts))))))

Maybe this could be added to cmake-ide?

atilaneves commented 5 years ago

What's the use case for having a space in the option?

andrmuel commented 5 years ago

In my case, there is CMake option that accepts multiple file names, separated by a space.

On 20/10/2018 19.07, Atila Neves wrote:

What's the use case for having a space in the option?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/atilaneves/cmake-ide/issues/181#issuecomment-431599913, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcLoss297JPzTafF4anb6thJtWKmuoLks5um1hrgaJpZM4XWNHY.

atilaneves commented 5 years ago

I looked at your original example again and -DCONF_FILES=foo.conf bar.conf won't work at all with clang. It'll be just one #define (foo.conf). I don't know about your CMake option but unless there are quotes around the space-delimited list, that won't work either.

Are you using an old version of cmake-ide? It doesn't use split-string but split-string-and-unquote. I explained the history why in my comment on issue #177

andrmuel commented 5 years ago

I was using cmake-ide-20180713; I see the split-string-and-unquote only in cide--split-command (which as I understand is not relevant for cmake-ide-cmake-opts).

I actually started out with something like -DCONF_FILES=\"foo.conf bar.conf\", but the only way I could actually get it to work is as described above.

The actual example I'm working with is the Zephyr OS (https://github.com/zephyrproject-rtos/zephyr), and my actual config is

(setq-local cmake-ide-cmake-opts '("-GNinja" "-DBOARD=nrf52840_pca10056" "-DCONF_FILE=prj.conf overlay-bt.conf"))

Looking at Zephyr itself, I see in cmake/kconfig.cmake:

string(REPLACE " " ";" CONF_FILE_AS_LIST "${CONF_FILE}")

Maybe this is the reason why I don't just get a #define foo.conf.

On 22/10/2018 14.11, Atila Neves wrote:

I looked at your original example again and |-DCONF_FILES=foo.conf bar.conf| won't work at all with clang. It'll be just one |#define| (foo.conf). I don't know about your CMake option but unless there are quotes around the space-delimited list, that won't work either.

Are you using an old version of |cmake-ide|? It doesn't use |split-string| but |split-string-and-unquote|. I explained the history why in my comment on issue #177 https://github.com/atilaneves/cmake-ide/issues/177

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/atilaneves/cmake-ide/issues/181#issuecomment-431831168, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcLoqsbHCxDGaDCGoPCxtCjYEwBO8Oeks5uncPygaJpZM4XWNHY.

atilaneves commented 5 years ago

Sorry, I misread your issue. I now understand what you mean.

The problem is that cmake-ide-cmake-opts should never have been a string but instead a list of strings. I didn't add that one and didn't think it through when I merged it.