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

Convert to GNU Octave code style #116

Closed
8 of 10 tasks
apjanke opened this issue Jan 30, 2024 · 6 comments
Closed
8 of 10 tasks

Convert to GNU Octave code style #116

apjanke opened this issue Jan 30, 2024 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@apjanke
Copy link
Owner

apjanke commented Jan 30, 2024

Let's convert Tablicious fully over to Octave's conventional code style.

  • Specific endif/endfor/endfunction/endxxx instead of plain end.
  • Use ! instead of ~ for negation.
  • Use () around if, while, and switch predicates.
  • # for comments (# for inline comments, ## for block comments).
  • Remove trailing whitespace from all lines.
  • 2-space indents.
  • Block comments (including one-liners) use ## instead of #.
  • String literals use "..." unless there's a specific reason for '...'.
  • Anything else in the Octave style guilde, unless I really hate it.
    • I'm passing on end_try_catch, just bc that looks gross to me.

Open questions:

  • Should the LHS of multi-argout [a,b,c] = ... assignments have spaces after commas like [a, b, c] = ...?

Don't care about Matlab compatibility here, because there's not much use case for running Tablicious code under Matlab at all.

It's fine to do code-style-only commits or PRs for this. And IMHO preferable: I'd rather get this all over with at once and have the whole Tablicious code base in consistent Octave style.

But wait until pr0m1th3as is done with this PR work before modifying table.m.

TODO

  • Re-read the Octave style guilde for more items.
  • [in-progress] Convert all code to Octave style.
  • Add .editorconfig with supporting settings, including trailing whitespace stripping.

References

@apjanke apjanke self-assigned this Jan 30, 2024
@apjanke apjanke added the bug Something isn't working label Jan 30, 2024
@github-project-automation github-project-automation bot moved this to Needs triage in Octave-Tablicious Jan 30, 2024
@apjanke apjanke added this to the 0.4.0 milestone Jan 30, 2024
@apjanke
Copy link
Owner Author

apjanke commented Jan 31, 2024

Open question: for multiple LHS variables in an assignment like [a,b,c] = ..., should there be spaces after those commas? I'm guessing yes, because in the array-concatenation [...] syntax it uses spaces, there's spaces after commas in function calls, and everywhere else except in x(a,b,c) or x{a,b,c} indexing calls that I can see, it uses spaces. But this isn't addressed directly in the GNU Octave style guide as far as I can tell.

@apjanke
Copy link
Owner Author

apjanke commented Jan 31, 2024

I did a rough draft bulk conversion for this on branch "WIP/swift-style". It addresses most of the items noted in the description, except for converting the generic ends for try/catch blocks; "end_try_catch" is disgusting and I can't bring myself to type that. I'll consider a PR if someone else wants to do it, though.

image

This is a multi-commit WIP work stream, and will be rebased and squashed before merging.

@apjanke
Copy link
Owner Author

apjanke commented Feb 3, 2024

I merged those style changes from the "rough draft" plus a couple others to main. That covers most of this, except for the (...) around if/while predicates, and other stuff that may be in the Octave style guide but I don't remember. I'll do a reread through the style guide before closing this.

@apjanke
Copy link
Owner Author

apjanke commented Feb 4, 2024

Octave style guide re-read notes

"It is customary to prefix the error message with the name of the function that generated it." But it doesn't mention using an actual error or warning identifier (the separate arg).

"For functions that are not present in Matlab, favor the use of underscores. For example, base64_decode, common_size, or compare_versions." I'm using camelCase instead.

Under Naming, outside "Function names" it says "Do not use mixed case (a.k.a. CamelCase) names." It doesn't mention class naming at all.

"Always use double quotes for strings and characters rather than the Matlab single quote convention." Except for strings containing double quotes or backslashes.

"Enclose the condition of an if, while, until, or switch statement in parentheses, as in C", but not for for statements, including the RHS of the for x = ....

"The negation operator is written with a space between the operator and its target, e.g., ! A." Oops! I'm not doing this.

"Comments that start with a double sharp-sign, ##, are stand-alone comments that occupy an entire line." They use this even for one-line comments; those are still considered "block comments". I'm not doing this.

Things they don't cover

  • Indentation level for statement continuations, like a foo(x, y, ... \n function call that's continued on the next line.
  • Anything about classes.
  • Anything about packages/namespaces.
  • Whitespace lines immediately inside properties, methods, and classdef blocks?
  • Spaces after commas in [x,y,z] = ... multi-argout LHS?

@apjanke
Copy link
Owner Author

apjanke commented Feb 4, 2024

Did some more style changes on the WIP/preallocation-ctor-form branch. Think I got all the big ones now, except:

  • Block comments (including one-liners) use ## instead of #.
  • String literals use "..." unless there's a specific reason for '...'.

I'm a little tired; don't think I'll get to those soon.

@apjanke
Copy link
Owner Author

apjanke commented Feb 4, 2024

I'm going to postpone doing the ## block comments and "..." double-quoted string literals until later. Closing as Fixed.

@apjanke apjanke closed this as completed Feb 4, 2024
@github-project-automation github-project-automation bot moved this from Needs triage to Closed in Octave-Tablicious Feb 4, 2024
@apjanke apjanke changed the title Convert to Octave code style Convert to GNU Octave code style Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Closed
Development

No branches or pull requests

1 participant