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

Inherit psysh history #3

Merged
merged 4 commits into from Jun 2, 2017

Conversation

@zonuexe
Copy link
Member

zonuexe commented Jun 1, 2017

refs #1 by @Fuco1

I'm not good at English. Please point out my unclear sentences.

psysh.el Outdated
"psysh")))

(defun psysh--load-history (path n)
"Load input histories by PATH and return N elements."

This comment has been minimized.

Copy link
@Fuco1

Fuco1 Jun 1, 2017

Member

History as a "list of past choices" is always singular, histories means "multiple stories", e.g. "Rome, Greece and Ancient China have interesting histories".

This comment has been minimized.

Copy link
@Fuco1

Fuco1 Jun 1, 2017

Member

Replace by with from, i.e. "from PATH" (we load from somewhere).

By in this context would mean "PATH" is some tool we use to do the loading, which is not the case.

This comment has been minimized.

Copy link
@zonuexe

zonuexe Jun 2, 2017

Author Member

I see. The history we are seeing this time comes from a single file 😄

psysh.el Outdated
do (forward-line -1)))))

(defun psysh--insertion-history-lines (histories)
"Insert history `HISTORIES' lines to comint-input-ring."

This comment has been minimized.

Copy link
@Fuco1

Fuco1 Jun 1, 2017

Member

Same as above, I think it will be enough to then say just Insert HISTORY lines to ... and rename the variable accordingly.

@Fuco1

This comment has been minimized.

Copy link
Member

Fuco1 commented Jun 1, 2017

Awesome job! Nothing much to add to the code, I have left some comments on English as you requested.

@Fuco1

This comment has been minimized.

Copy link
Member

Fuco1 commented Jun 1, 2017

By the way your English is fine! The issues above are rather minor and the content was understandable. But it always pays to improve, right? :)

@zonuexe zonuexe merged commit 9fe4c82 into master Jun 2, 2017
@zonuexe zonuexe deleted the feature/psysh-history branch Jun 2, 2017
@zonuexe

This comment has been minimized.

Copy link
Member Author

zonuexe commented Jun 2, 2017

@Fuco1 Thank you for your reviewing!

But it always pays to improve, right? :)

🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.