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.

Sign up for GitHub

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

Jump to bottom

followup #17398: getTempDir, getConfigDir now do not have trailing DirSep #17402

Merged
merged 2 commits into from
Mar 18, 2021
Merged

followup #17398: getTempDir, getConfigDir now do not have trailing DirSep #17402

merged 2 commits into from
Mar 18, 2021

Conversation

timotheecour
Copy link
Member

followup #17398

@timotheecour timotheecour mentioned this pull request Mar 17, 2021
Merged
1 task
@timotheecour timotheecour marked this pull request as ready for review March 17, 2021 08:49
@Araq
Copy link
Member

Araq commented Mar 17, 2021

This breaks the "inim" package... Do we really want to break people's code because one guy on the forum found an inconsistency that prevented him from using a bad solution? I think it's a terrible way to develop a project.

@vercingetorx
Copy link
Contributor

vercingetorx commented Mar 17, 2021 •
edited
Loading

@Araq That was just a test case to highlight the inconsistencies between the procs. My argument against keeping it inconsistent because it "discourages bad code" is that the inconsistency itself is bad code.

timotheecour added a commit to timotheecour/INim that referenced this pull request Mar 18, 2021
@timotheecour
fix usages of getTempDir, refs nim-lang/Nim#17402
485e202
@timotheecour timotheecour mentioned this pull request Mar 18, 2021
Merged
@timotheecour
Copy link
Member Author

I'm fixing inim in inim-repl/INim#117;
using:

getTempDir() & "inim_" & $uniquePrefix & ".nim"

is just asking for trouble

@Varriount
Copy link
Contributor

I have to wonder if randomly adding/removing separators as part of the actual logic wouldn't lead to more reliable code...
Whenever I've have written logic that depended on the presence/absence of trailing separators, it usually broke at some point...

0atman pushed a commit to inim-repl/INim that referenced this pull request Mar 18, 2021
@timotheecour
fix usages of getTempDir, refs nim-lang/Nim#17402 (#117)
53a18a5
@0atman
Copy link
Contributor

0atman commented Mar 18, 2021

Thank you for your consideration @timotheecour & @Araq! The PR's merged in (thank you for that), that's a go from our side!

@timotheecour timotheecour force-pushed the pr_followup_17398_no_DirSep branch from b8b67ad to daa307a Compare March 18, 2021 09:17
@Araq Araq merged commit 2356d06 into nim-lang:devel Mar 18, 2021
@timotheecour timotheecour deleted the pr_followup_17398_no_DirSep branch March 18, 2021 18:15
c-blake added a commit to c-blake/cligen that referenced this pull request Mar 21, 2021
@c-blake
Adapt to nim-lang/Nim#17402
5a6dda9
c-blake added a commit to c-blake/lc that referenced this pull request Mar 21, 2021
@c-blake
Adapt to nim-lang/Nim#17402
2fc0ecc
c-blake added a commit to c-blake/procs that referenced this pull request Mar 21, 2021
@c-blake
Adapt to nim-lang/Nim#17402
0b7bf3b
ringabout pushed a commit to ringabout/Nim that referenced this pull request Mar 22, 2021
@timotheecour @ringabout
followup nim-lang#17398: getTempDir, getConfigDir now do not have…
16cb728 
… trailing DirSep (nim-lang#17402)

* followup nim-lang#17398: `getTempDir`, `getConfigDir` now do not have trailing DirSep

* fix test
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
@timotheecour @ardek66
followup nim-lang#17398: getTempDir, getConfigDir now do not have…
198d2a0 
… trailing DirSep (nim-lang#17402)

* followup nim-lang#17398: `getTempDir`, `getConfigDir` now do not have trailing DirSep

* fix test
@timotheecour timotheecour mentioned this pull request May 14, 2021
Closed
narimiran added a commit to narimiran/Nim that referenced this pull request Jul 12, 2021
@narimiran
Revert "followup nim-lang#17398: getTempDir, getConfigDir now do …
3f9e644 
…not have trailing DirSep (nim-lang#17402)"

This reverts commit 2356d06.
Araq pushed a commit that referenced this pull request Jul 18, 2021
@narimiran
Revert #17398 and #17402 (#18480)
8091545 
* Revert "followup #17398: `getTempDir`, `getConfigDir` now do not have trailing DirSep (#17402)"

This reverts commit 2356d06.

* Revert "fix #17393 getHomeDir and expandTilde should not include trailing `/` (#17398)"

This reverts commit bebf2ce.

* fix test
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
@narimiran @PMunch
Revert nim-lang#17398 and nim-lang#17402 (nim-lang#18480)
f5bd211 
* Revert "followup nim-lang#17398: `getTempDir`, `getConfigDir` now do not have trailing DirSep (nim-lang#17402)"

This reverts commit 2356d06.

* Revert "fix nim-lang#17393 getHomeDir and expandTilde should not include trailing `/` (nim-lang#17398)"

This reverts commit bebf2ce.

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0atman 0atman 0atman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@timotheecour @Araq @vercingetorx @Varriount @0atman