| 1 |
On 11/02/2014 06:16 PM, Andrés Martinelli wrote: |
| 2 |
> Hello there!! |
| 3 |
> I am working on a terminal spreadsheet based on "sc", but with some |
| 4 |
> adds like undo/redo.. |
| 5 |
> you can find it here: |
| 6 |
> |
| 7 |
> https://github.com/andmarti1424/scim |
| 8 |
> |
| 9 |
> Any new ideas and/or contribution is always welcome! |
| 10 |
> Thanks! |
| 11 |
> |
| 12 |
> -- |
| 13 |
> Andrés M. |
| 14 |
|
| 15 |
If I can offer some constructive criticism based on my short experience |
| 16 |
helping Walter: |
| 17 |
|
| 18 |
* Your build process could use some work; you shouldn't be hard-coding |
| 19 |
variable values like LN and CC in a Makefile, these are handled by make. |
| 20 |
Your Makefile could be shortened by ~150 lines by relying on built-in |
| 21 |
rules and using some built-in make expressions to list your source files |
| 22 |
as well. |
| 23 |
* A bunch of your .c or .h files are marked as executable... why? |
| 24 |
* No install command. Not really a huge deal as only one important file |
| 25 |
is produced (src/scim), but would be nice to have |
| 26 |
|
| 27 |
If you're unfamiliar with make and have no immediate plans to switch to |
| 28 |
either the autotools or cmake, I would be more than willing to make some |
| 29 |
changes to your Makefile and open a pull request. If you would like to |
| 30 |
email me personally with questions about make, feel free to do that as well. |
| 31 |
|
| 32 |
There are a couple things I saw in the code as well: |
| 33 |
|
| 34 |
* system("echo -n 'Press enter to return.'") |
| 35 |
* Lot of ignored return values on functions that you should check, like |
| 36 |
write() and fgets() |
| 37 |
|
| 38 |
If these were in the sc code and you know about them but just haven't |
| 39 |
gotten to fixing them yet, no worries. |
| 40 |
|
| 41 |
Alec |
Archives