Fix: Chop git suffix from LibLLVM::VERSION
#13699
Conversation
src/llvm/lib_llvm.cr
Outdated
| @@ -10,7 +10,7 @@ end | |||
| {% end %} | |||
| @[Link(ldflags: {{"`#{LibLLVM::LLVM_CONFIG} --libs --system-libs --ldflags#{" --link-static".id if flag?(:static)}#{" 2> /dev/null".id unless flag?(:win32)}`"}})] | |||
| lib LibLLVM | |||
| VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify}} | |||
| VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify.gsub(/git/,"")}} | |||
There was a problem hiding this comment.
For future reviewers: Note there is no equivalent to String#rchop for StringLiteral which might've been nice for this.
src/llvm/lib_llvm.cr
Outdated
| @@ -10,7 +10,7 @@ end | |||
| {% end %} | |||
| @[Link(ldflags: {{"`#{LibLLVM::LLVM_CONFIG} --libs --system-libs --ldflags#{" --link-static".id if flag?(:static)}#{" 2> /dev/null".id unless flag?(:win32)}`"}})] | |||
| lib LibLLVM | |||
| VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify}} | |||
| VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify.gsub(/git/,"")}} | |||
There was a problem hiding this comment.
Could be tweaked a bit:
| VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify.gsub(/git/,"")}} | |
| VERSION = {{ `#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify.gsub(/git$/, "") }} |
There was a problem hiding this comment.
Hm, actually there's no need for a regex:
| VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify.gsub(/git/,"")}} | |
| VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify.gsub("git","")}} |
There was a problem hiding this comment.
There is no StringLiteral#sub, either
There was a problem hiding this comment.
@straight-shoota There is, if you want to remove the git from the end of the string only.
There was a problem hiding this comment.
@HOMODELUNA WDYT of making it work with the string instead of the redex as proposed by @straight-shoota?
There was a problem hiding this comment.
That wouldn't work either, the macro StringLiteral#gsub requires a RegexLiteral first argument
There was a problem hiding this comment.
And of course, moving the .gsub or .sub outside {{ ... }} isn't an option as LibLLVM::VERSION's initializer must be a literal
There was a problem hiding this comment.
Okay, so we leave everything as it is. Sorry for all the confusion 🙇
There was a problem hiding this comment.
IMO $ should be added, still.
|
This still needs formatting |
|
ah good catch |
straight-shoota
changed the title
LibLLVM::VERSION
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
No description provided.