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

Remove dependency on grep in README + Improve findstr command #121

Merged
merged 1 commit into from Jan 27, 2022
Merged

Remove dependency on grep in README + Improve findstr command #121

merged 1 commit into from Jan 27, 2022

Conversation

maximecharriere
Copy link
Contributor

Hello, it's me again.
First of all, I have removed the dependency on grep in the README.

I have also slightly improved yesterday's PR #120.
The current solution with findstr is fully functional, but it has a little difference with the solution using grep.
Due to a strange behaviour of execute_process(COMMAND ...) on Windows, the following two lines are returned by findstr, whereas only the first one is returned if the command is run directly in the shell.

#define NABO_VERSION "1.0.7"
#define NABO_VERSION_INT 10007

However, this did not cause any problem as string(REGEX REPLACE ...) still managed to find the correct version.

I asked the question on Stackoverflow (link), and the solution off this PR was found.

@ethzasl-jenkins
Copy link

Can one of the admins verify this patch?

@pomerlef
Copy link
Collaborator

ok to test

@pomerlef
Copy link
Collaborator

I'm a bit blind on the support for Windows as it's not included in our CI, as long as it doesn't break the other system support, and it looks raisonnable, I'll let you investigate.

@pomerlef pomerlef merged commit c925c47 into norlab-ulaval:master Jan 27, 2022
@maximecharriere maximecharriere deleted the windows/grep branch January 27, 2022 19: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

3 participants