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

fix pathSubs for windows: --outdir:@projectpath/foo works cross platform, and without quoting needed #13407

Closed

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Feb 13, 2020

before PR

  • nim c --outdir:'$projectpath'/foo main.nim works on posix, but not on windows inside exec (inside nims program) or execCmd (inside nim program) or on windows command prompt (only works on windows git bash cmd prompt)
  • nim c --outdir:$projectpath/foo main.nim works on windows cmd prompt, but not on posix, and not on windows git bash prompt

likewise with other path substitutions supported by pathSubs

after PR

nim c --outdir:@projectpath/foo main.nim works everywhere (posix and windows, nim or nims or on cmdline), and doesn't require any quoting

note

inc(j)
if isNumber == 1:
let idx = if not negative: k-1 else: a.len-k
else:
Copy link
Member Author

@timotheecour timotheecour Feb 13, 2020

Choose a reason for hiding this comment

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

note to reviewer: you can check the diff manually, but this is pure re-indentation below here

@@ -2623,69 +2623,80 @@ proc findNormalized(x: string, inArray: openArray[string]): int =
proc invalidFormatString() {.noinline.} =
raise newException(ValueError, "invalid format string")

proc addf*(s: var string, formatstr: string, a: varargs[string, `$`]) {.
noSideEffect, rtl, extern: "nsuAddf".} =
proc addfCustom*(s: var string, formatstr: string, subs = {'$'}, a: varargs[string, `$`]) {.
Copy link
Member

Choose a reason for hiding this comment

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

Just make an addfCustom for the compiler only then...

@stale
Copy link

stale bot commented Apr 24, 2021

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.

@stale stale bot added the stale Staled PR/issues; remove the label after fixing them label Apr 24, 2021
@stale stale bot closed this May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Staled PR/issues; remove the label after fixing them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants