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

Changed 'writefile' to 'save-file' #133

Merged
merged 24 commits into from
Jan 14, 2019
Merged

Conversation

Kinnardian
Copy link
Contributor

Changed writefile to save-file to be consistent with save-table / write-table.

@Kinnardian Kinnardian mentioned this pull request Dec 25, 2018
@@ -2898,7 +2899,7 @@ Useful in higher-order functions, or to index into lists, strings, tables, etc."
(= (savers* ',var) (fn (,gv) (,save ,gv ,file)))))))

(mac diskvar (var file)
`(fromdisk ,var ,file nil readfile1 writefile))
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, now I'm wondering if we should also rename readfile1 to load-file and so on.

But that piles on the incompatibilities. Others may object. What do you think, @hjek?

akkartik and others added 20 commits December 31, 2018 22:04
Ported from https://docs.racket-lang.org/reference/regexp.html

Thanks Kinnard Hockenhull for the questions and feedback about regular
expressions in Arc.
```
warning: connecting to bitbucket.org using legacy security technology (TLS 1.0)
```

https://www.mercurial-scm.org/wiki/SecureConnections
It had been broken since commit 95ff093 in Nov 2017, when I moved it from
a library into arc.arc. Unfortunately I moved it to the wrong place, before
assignment was defined. Not only did it break `nappend`, it also yielded
cryptic error messages when attempting to use `nappend`.

`nappend` was originally introduced because I wanted it for spliceable
lists. Unfortunately I never wrote tests for it running on regular lists,
hiding the bug for over a year. Now starting to add tests.

Thanks Kinnard Hockenhull for reporting this issue.
This tendency to misuse `ret` is one area where I've been bitten a few
times by Lisp's syntactic regularity.
… enabled, for better reporting of read-time syntax errors. Thanks to kinnard for porting this out at <http://arclanguage.org/item?id=20943>. Enable similar line-counting for ac.rkt's `acompile` and `test` functions.
…test.arc" instead of the correct name "tests.arc".
@Kinnardian
Copy link
Contributor Author

Breaks temstore

(def temstore (tem val file)

@akkartik
Copy link
Member

Good catch! I also see a call to writefile in apps/news/news.arc.

Worse, none of our tests catch this. That's something to think about. I'd like to test reading/writing templates.

Could you fix both these callsites and let me know? Let's get this merged. We can always tweak or roll back things later. No need to add any tests in this PR.

@Kinnardian
Copy link
Contributor Author

All in one new PR? I think I can pull that off.

@akkartik
Copy link
Member

You can just add to this PR. Just do a new commit and git push. You should see this page update.

@akkartik
Copy link
Member

Oh, before you do, run this:

$ git pull --rebase

@Kinnardian
Copy link
Contributor Author

I did something wrong . . .

@akkartik
Copy link
Member

Very strange. It looks ok on the commandline, but GitHub's UI seems to be confused.

@akkartik
Copy link
Member

Let's try merging it and see what happens.

@akkartik akkartik merged commit 79a09ca into arclanguage:master Jan 14, 2019
@Kinnardian
Copy link
Contributor Author

It might be because my remote is not my fork:

$ git remote -v
origin	https://github.com/arclanguage/anarki/ (fetch)
origin	https://github.com/arclanguage/anarki/ (push)

¿

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

5 participants