Closed GoogleCodeExporter closed 9 years ago
Yes, feel free to add comments like this to the source code.
It surely would be useful.
Maybe an even more structured format would help to make the code better
searchable.
In phpLiteAdmin code, I usually search for the piece of code where actions are
performed. e.g. where the "column_edit" form code is and where the
"column_edit&confirm=1" action perform part is. Currently I use the editor
search function a lot to find these places. But action names like "column_edit"
occur at several places where the action is not really performed and the form
is not generated. So I have to search for the next occurrence several times to
get where I am heading for.
So maybe comments like this would be very useful because you could find the
place you are looking for very fast:
// - form to edit a column (ACTION column_edit UNCONFIRMED)
and where the action is really performed:
// - edit a column (ACTION column_edit CONFIRMED)
So you can search for "column_edit UNC" or "column_edit CONF" and you are
directly where you want to go.
Although better comments are good and stuff like this will help for the time
being, we should in the long-run really use more OOP. This would make finding
the right place a lot simpler, so you can just look for the edit() method of
the Column Class for example.
Original comment by crazy4ch...@gmail.com
on 15 Jan 2014 at 9:24
> Although better comments are good and stuff like this will help
> for the time being, we should in the long-run really use more OOP.
That's why I wouldn't spend to much time on this. I would just make sure that
all the important sections have a comment 'heading' and we know (more or less)
where stuff is.
Patch soon ;)
Original comment by dreadnaut
on 16 Jan 2014 at 12:26
Here's is the diff for the new comments. The syntax I used:
1) map comments start with '//- '
2) larger html chunks are marked with 'HTML: '
3) switch values are written as '=string'
The last point should make actions easier to find in-editor; let me know if it
works for you.
Also attached is the source map that comes from running
grep -n //- index.php
Original comment by dreadnaut
on 17 Jan 2014 at 12:44
Attachments:
ok, cool. Only thing I noticed is a typo:
//- Delete and existing database
should be
//- Delete an existing database
Feel free to commit.
Original comment by crazy4ch...@gmail.com
on 17 Jan 2014 at 1:17
This issue was closed by revision r458.
Original comment by dreadnaut
on 17 Jan 2014 at 1:20
Original issue reported on code.google.com by
dreadnaut
on 11 Jan 2014 at 7:41