pereorga / minimalist-web-notepad

Minimalist Web Notepad
https://notes.orga.cat
1.01k stars 276 forks source link

remove $length for now #20

Closed qex closed 6 years ago

qex commented 6 years ago

Yeah, I'd willing to~

pereorga commented 6 years ago

Sorry for merging too fast. I've also applied some cosmetics

qex commented 6 years ago

Never mind, I saw you still leave $length there?

pereorga commented 6 years ago

Yes, I left it for now. I'm a bit unsure of what's best. What do you think?

qex commented 6 years ago

I've thinking about this for half an hour.

the conclusion for now is, leave that 5 there, and write a comment like if you need to change the default name length of note, change that 5 below.

pereorga commented 6 years ago

What about renaming the variable to $default_name_length. And then without the comment.

qex commented 6 years ago

I've done that before, put this line above $characters and below if.

It is still an extra variable for rarely executed if condition, but it is useful than my first $size.

qex commented 6 years ago

After all in the original code this variable is not a explicit config also.

If we need to add some customization, write it as $name_length, not default because it is modifiable.

In other way if we need to keep the original minimalist type, leave a 5 there.

pereorga commented 6 years ago

If we need to add some customization, write it as $name_length, not default because it is modifiable.

Done.

In other way if we need to keep the original minimalist type, leave a 5 there.

Hard decision, but I like it now. Minimalist also means less comments. We can switch to const in the future, when we don't care about older PHP versions.

pereorga commented 6 years ago

I see what you meant about $randomString variable. Changed now.

qex commented 6 years ago

Yup, for now the lowest version to run the notepad is 5.0.0, limited by file_put_contents.

If we remove that, this pad can run on php 4.3.0, limited by file_get_contents, that is amazing.

that make the pad run almost everywhere. just upload, and run, that is how I love this project.

I think using variable as config is nothing bad, there's no need to force using constance isn't it?

pereorga commented 6 years ago

yes, it's OK :)

qex commented 6 years ago

Seems it is near 0 o'clock at your time ?

Time to sleep! I will continue PR this project when I got new ideas.

for example change print to echo, change <?php print $foo; ?> to <?=$foo?>, etc.

some optimization of no importance in total, and curl problem besides I think ?

pereorga commented 6 years ago

Cool, have a good day. And thanks!

qex commented 6 years ago

You're welcome. It is my pleasure to make a great project better~ :wink: