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

implement 'source' builtin command #18

Merged
merged 5 commits into from Mar 20, 2019
Merged

implement 'source' builtin command #18

merged 5 commits into from Mar 20, 2019

Conversation

ngklingler
Copy link
Contributor

Hey,

I noticed your shell does not have the source command as one of its builtins, which lets you source rc files during a session. It works like this: make a change to ~/.cicadarc, run source ~/.cicadarc, and any changes you made to your RC file will then be loaded. I was a pretty simple implementation, I just had to call the load_file function in the rcfile module. I hope you will incorporate this into your project.

@mitnk
Copy link
Owner

mitnk commented Mar 18, 2019

Hi @ngklingler, Thanks for the PR.

The source builtin should be more capable than just loading rcfile. It should also run cicada scripts (which is not supported yet, see the two big todo in readme).

But reloading refile is a useful feature. We can merge this PR with the following adjust:

  1. Check the input file, if it ends with "cicadarc", then reload the rc, otherwise, we print a hint that "scripting in cicada is still work in progress".

  2. Can you add source into src/completers/path.rs around line 168. So cicada completion knows this new builtin.

  3. Also a small item in Changelog would be great.

@ngklingler
Copy link
Contributor Author

Hey @mitnk

Good call on those points. I changed the source commmand to check that the specified file ends in "cicadarc" if it does, we load the file; if it doesn't then it throws an error that only loading from cicadarc files is supported and scripting is a WIP. I added the source command to the completer, and I updated the changelog to mark this change. Let me know if anything else should be done.

}
if !args[1].ends_with("cicadarc") {
println_stderr!("cicada: source command only supports cicadarc files");
println_stderr!("scripting in cicada is still a work in progress");
Copy link
Owner

Choose a reason for hiding this comment

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

I think here we should have a return 1; :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 23ec537

Wouldn't be coding if I didn't introduce an error :)

@mitnk mitnk merged commit 3165323 into mitnk:master Mar 20, 2019
@mitnk
Copy link
Owner

mitnk commented Mar 20, 2019

Looks great! Thanks again!

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

2 participants