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 line command, misc style adjustments #21

Merged
merged 6 commits into from
Feb 2, 2016

Conversation

phillid
Copy link
Contributor

@phillid phillid commented Jan 24, 2016

I have implemented a "naïve first attempt" at the line command in response to #6, permitting dodo to jump to specific line numbers. Immediately, I can see two issues:

  1. dodo may consume a lot of the file before finding the (n-1)th line break
  2. dodo will fseek to the beginning of the file, whereas relative line jumping should be possible, although less clear to implement and prone to more bugs

As you state in #6, we can put the responsibility for (1) onto the user, however I'm not sure if we can say the same about (2) or not. Some simple testing has been added for the line command but we are still lacking the ability to test failure conditions, which limits the amount of testing we can add.

This command makes me wonder if we should also extend the byte command to allow relative byte jumps. It's useful to jump to a certain line, but it is also then useful to jump to a certain section of that line, and currently the only way of doing this would be e/foo/w/foo/ to jump over foo at the start of a line, which is far from ideal.


I have also aligned some if blocks with the style used throughout the rest of the source and added a little extra something to the interactive prompt to show the current cursor position in bytes .

@chrisosaurus
Copy link
Owner

@phillid I think we need both relative and absolute offsets for all commands, completely agree.
possible syntax:

bn (absolute, from start of file)
b-n (relative back)
b+n (relative forward)

we could then centralise this n, -n, +n parsing and eval logic (e.g. calc_num is given the current number (byte/line) and the n, -n, or +n and returns the new number)

@@ -324,8 +324,9 @@ struct Instruction * parse_print(char *source, size_t *index){
*/
if( isdigit(source[*index]) ){
ret = parse_number(i, source, index);
if(ret == 0)
if( ret == 0 ){
Copy link
Owner

Choose a reason for hiding this comment

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

thanks for this, I need to write a style guide at some point.
I do like

if( cond ){
    body
}

and

if( cond ){
    body
} else {
    body
}

👍

@chrisosaurus
Copy link
Owner

Everything looks correct and sane, all tests pass, and style is good.

Accepting PR, @phillid please look over my comments and let me know what you think of them.

chrisosaurus added a commit that referenced this pull request Feb 2, 2016
Implement line command, misc style adjustments
@chrisosaurus chrisosaurus merged commit 52f622e into chrisosaurus:master Feb 2, 2016
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.

2 participants