From e975e546e1ef36099abd268ea5715a1888037642 Mon Sep 17 00:00:00 2001 From: Jakub Vrana Date: Sun, 23 Mar 2025 13:33:59 +0100 Subject: [PATCH] Docs: improve style thanks to ChatGPT --- CONTRIBUTING.md | 4 +- README.md | 15 +++-- SECURITY.md | 6 +- developing.md | 147 ++++++++++++++++++++++++------------------------ 4 files changed, 85 insertions(+), 87 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f92d7de3..6889d844 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,3 +1,3 @@ - Reproducible [bug reports](https://github.com/vrana/adminer/issues/new?template=bug_report.md) are warmly welcomed. -- [Feature requests](https://github.com/vrana/adminer/issues/new?template=BLANK_ISSUE) are also fine but I'm quite picky about what to accept to Adminer. Please don't be offended if I close the issue as Not Planned, especially if it can be achieved with a plugin. -- [Pull requests](https://github.com/vrana/adminer/pulls) both for bugs and simple features are also fine. Before doing anything more complicated, get familiar with [Adminer philosophy](/developing.md). +- [Feature requests](https://github.com/vrana/adminer/issues/new?template=BLANK_ISSUE) are also fine, but I'm quite picky about what to accept into Adminer. Please don't be offended if I close the issue as "Not Planned," especially if it can be achieved with a plugin. +- [Pull requests](https://github.com/vrana/adminer/pulls) for both bug fixes and simple features are welcome. Before working on anything more complicated, get familiar with the [Adminer philosophy](https://github.com/vrana/adminer/blob/master/developing.md). diff --git a/README.md b/README.md index 079f297d..47fcf60e 100644 --- a/README.md +++ b/README.md @@ -1,17 +1,16 @@ # Adminer - -**Adminer** is a full-featured database management tool written in PHP. -It consists of a single file ready to deploy to the target server. +**Adminer** is a full-featured database management tool written in PHP. It consists of a single file ready to deploy to the target server. **Adminer Editor** offers data manipulation for end-users. -https://www.adminer.org/ +[Official Website](https://www.adminer.org/) +## Features - **Supports:** MySQL, MariaDB, PostgreSQL, CockroachDB, SQLite, MS SQL, Oracle - **Plugins for:** Elasticsearch, SimpleDB, MongoDB, Firebird, ClickHouse, IMAP - **Requirements:** PHP 5.3+ ## Screenshot -![screenshot](https://www.adminer.org/static/screenshots/table.png) +![Screenshot](https://www.adminer.org/static/screenshots/table.png) ## Installation If downloaded from Git then run: `git submodule update --init` @@ -27,8 +26,8 @@ If downloaded from Git then run: `git submodule update --init` - `tests/*.html` - Katalon Recorder test suites ## Plugins -There are [several plugins](plugins/) distributed with Adminer and there are also many user-contributed plugins linked from https://www.adminer.org/plugins/. -To use a plugin, simply upload it to `adminer-plugins/` next to `adminer.php`. You can also upload plugins for drivers (e.g. `elastic.php`) here. +There are [several plugins](/plugins/) distributed with Adminer, as well as many user-contributed plugins linked on the [Adminer Plugins page](https://www.adminer.org/plugins/). +To use a plugin, simply upload it to the `adminer-plugins/` directory next to `adminer.php`. You can also upload plugins for drivers (e.g., `elastic.php`) in this directory. ``` - adminer.php @@ -40,7 +39,7 @@ To use a plugin, simply upload it to `adminer-plugins/` next to `adminer.php`. Y - adminer-plugins.php ``` -Some plugins require configuration. To use them, create a file `adminer-plugins.php`. You can also specify the loading order here. +Some plugins require configuration. To use them, create a file named `adminer-plugins.php`. You can also specify the loading order in this file. ```php register("tableName", $callback)` and then `$hooks->call("tableName")` but I like the fact that I can simply call `$adminer->tableName()` in Adminer code. Supporting this syntax with hooks would mean just moving the repetitive code elsewhere. +A more common method for extending Adminer is the [`Plugins`](/adminer/include/plugins.inc.php) class. Although its code is somewhat repetitive, it effectively allows developers to create plugins without extending `Adminer` directly. Since developers are accustomed to defining classes with methods, this approach has a low entry barrier. I considered a hook system (`$hooks->register("tableName", $callback)`, then `$hooks->call("tableName")`), but I prefer the direct call syntax (`$adminer->tableName()`). Implementing hooks while maintaining this syntax would simply relocate repetitive code. -## Code style +## Code Style -Adminer uses quite strict code [style](/phpcs.xml) which might be perhaps slightly unusual. E.g. doc-comments are not indented by one space which is because some editors start the next line with a space if you hit Enter after `*/` (e.g. VS Code). I'm not very attached to a particular code style and I'm open to changes but all code must look the same. So if you want to change the code style then you need to adapt all existing code to it. +Adminer follows a strict [coding style](/phpcs.xml), though some choices may seem unusual. For instance, doc-comments are not indented by one space because some editors (e.g., VS Code) insert a space when pressing Enter after `*/`. -There's no rule about using `"` or `'`. Most code uses `"` because it's more versatile e.g. if you decide to use a variable in the string later. I use it even in cases where it's unlikely that a variable would be used later (e.g. `$_GET["table"]`) just because I have an editor snippet to insert this. `'` is used mostly with regular expressions and it is mandatory for extracting translations in `lang()`. +There is no enforced rule on `"` vs. `'`. Most code uses `"` because it's more flexible (e.g., embedding variables). Even in cases where variable interpolation is unlikely (e.g., `$_GET["table"]`), I still use `"` due to an existing editor snippet. `'` is primarily used for regular expressions and is required for extracting translations in `lang()`. -I don't use `"{$var}"` because it's longer. In the rare cases where `$var` couldn't be used in a string directly, I rather split the string: `"prefix$var" . "suffix"`. +I avoid `"{$var}"` because it is longer. In rare cases where `$var` cannot be used directly within a string, I prefer splitting the string (`"prefix$var" . "suffix"`). -Never use `$_REQUEST`. Make your mind about the right place for the param and access it there. +Never use `$_REQUEST`. Decide where the parameter belongs and access it accordingly. -I'm not very happy about naming style. PHP's global functions use snake_case so I use it too in functions and variable names. The MySQLi's `Db` class extends the `mysqli` class so it uses snake_case for its methods too. But I prefer camelCase which is used in methods of other classes and their parameters. So it's not very consistent and sometimes you pass `$table_status` to a method accepting `$tableStatus`. The best solution is to use one word for everything which is not very practical. Some pages use capitals for the main object, e.g. `$TABLE` which I don't like very much but it shines nicely in the code. +I am not entirely satisfied with the naming style. PHP global functions use `snake_case`, so I use it for functions and variables. MySQLi’s `Db` class extends `mysqli`, so it also uses `snake_case`. However, I prefer `camelCase` for method names and parameters so I use it in other classes. This inconsistency sometimes results in passing `$table_status` to a method expecting `$tableStatus`. The best approach would be to use single-word names, though this is impractical. Some pages use uppercase for main object (e.g., `$TABLE`), but I dislike this despite its visibility. -Code after `if` and loops must be wrapped to `{}` blocks. They are removed in minification. `else if` is forbidden, use `elseif`. +Code within `if` statements and loops must always be wrapped in `{}` blocks. These are removed during minification. `else if` is forbidden; use `elseif` instead. -I use empty lines to split code blocks but perhaps slightly less than usual. I have an editor shortcut to jump between empty lines and I use it primarily to jump between functions if a file has them. Lines with lone `}` divide the code optically well enough for me. +I use empty lines sparingly to separate code blocks. My editor shortcut jumps between empty lines, primarily for navigating functions. Lines containing only `}` naturally divide the code visually. -Well used ternary operators make the code more readable and shorter. However, Adminer code sometimes overuses them. +Well-used ternary operators enhance readability, but they are sometimes overused in Adminer. ```php -// I find this more readable and less repetitive: +// Preferred $title = ($update ? lang('Save and continue edit') : lang('Save and insert next') ); -// Than this: +// Less desirable if ($update) { $title = lang('Save and continue edit'); } else { // If you change else to elseif in the future then $title may stay uninitialized @@ -73,17 +73,17 @@ if ($update) { } ``` -Adminer has a line length limit 250 which is ridiculous. All lines fit my screen but I still want to make them shorter. Perhaps 150 would be more reasonable but I also hate wrapping lines at random places - they must be wrapped at logical blocks which often requires at least a small refactoring. These refactorings introduced bugs in the past so I'm hesitant to do them just to fit some arbitrary limit. +Adminer has an excessive line length limit of 250 characters. While all lines fit my screen, I prefer shorter lines. A limit of 150 would be more reasonable, but wrapping lines at arbitrary points is unacceptable. Proper line wrapping often requires refactoring, which has caused bugs in the past, so I hesitate to make changes purely for line length. ## Comments -All functions have doc-comments but I hate repetition so e.g. the `Db` methods are documented only in [mysql.inc.php](/adminer/drivers/mysql.inc.php) and not in the other drivers. Parameter names are not repeated in `@param`, only the type and description is there based on the order. Doc-comment is imperative ("Get" not "Gets"), starts with a capital letter and doesn't end with a fullstop +All functions have doc-comments, but redundancy is avoided. For example, `Db` methods are documented only in [mysql.inc.php](/adminer/drivers/mysql.inc.php), not in other drivers. `@param` tags include only type and description, based on order. Doc-comments are imperative ("Get" instead of "Gets"), start with a capital letter, and do not end with a period. -Inline commets are useful e.g. to link specifications but they are usually avoided to explain the code which should be self-explanatory. Inline comments start with a small letter but I'm not very happy about it. They don't end with a fullstop. +Inline comments are useful for linking specifications but are generally avoided for explaining self-explanatory code. They start with a lowercase letter and do not end with a period, though I am not entirely happy with this convention. -## Error handling +## Error Handling -Adminer strictly initializes all variables before use which is [checked](https://github.com/vrana/php-initialized). However, Adminer relies on the default value of uninitialized array items. This leads to much more readable code, consider e.g. this code: +Adminer strictly initializes all variables before use, which is [verified](https://github.com/vrana/php-initialized). However, Adminer relies on the default value of uninitialized array items. This approach leads to more readable code. Consider the following examples: ```php // Adminer style @@ -99,92 +99,91 @@ if (extension_loaded("mysqli") && ($_GET["ext"] ?? "") != "pdo") if (extension_loaded("mysqli") && idx($_GET, "ext") != "pdo") ``` -Treating undefined variables as empty was a big progress from the C language where they contain random data. Sadly, developers abused this feature which led PHP to issue first notices and then warnings in this case. Adminer [silences](https://github.com/vrana/adminer/blob/v5.0.6/adminer/include/errors.inc.php) these errors. In projects, where I'm forced to check array key existence before using it, I quickly create a function like this. +Treating undefined variables as empty was a significant improvement over the C language, where they contained random data. Unfortunately, developers abused this feature, leading PHP to issue first notices and later warnings. Adminer [silences](/adminer/include/errors.inc.php) these errors. In projects where I am required to check array key existence before usage, I quickly create a function like this: ```php function idx($array, $key, $default = null) { - // Note that this couldn't use isset() because idx(array(null), 0, '') would return a wrong value - return (array_key_exists($key, $array) ? $array[$key] : $default); + // Note: isset() cannot be used here because idx(array(null), 0, '') would return an incorrect value. + return array_key_exists($key, $array) ? $array[$key] : $default; } ``` -It would be possible to use such a function in Adminer but the code would still be less readable than the current approach. Using `isset` can lead to bugs such as in this code: `isset($rw["name"])` (I want to check if `$row` contains `name` but I've made a typo in the variable name which `isset` silences). `empty()` is even worse and it should be avoided in most cases. +Although it would be possible to use such a function in Adminer, the code would still be less readable than the current approach. Using `isset` can introduce bugs, such as in this case: `isset($rw["name"])`. Here, I intended to check if `$row` contains `name`, but a typo in the variable name is silently ignored. `empty()` is even worse and should be avoided in most cases. -Adminer uses `@` only in cases where a possible error is unavoidable, e.g. when writing to files (even if you check if the file is writable then there's a race condition between the check and the actual write). +Adminer uses `@` only where an error is unavoidable, such as when writing to files. Even if you check whether a file is writable, a race condition exists between the check and the actual write operation. ## Escaping -There's no auto-escaping in Adminer. When printing untrusted data (including e.g. table names), you must use `h()` which is a shortcut for `htmlspecialchars` with escaping also `"` and `'`. It would be nice to have some template system but it would have to be powerful enough to support streaming. Adminer needs to print data immediately to display at least partial results when some query is slow. +Adminer does not implement automatic escaping. When printing untrusted data (including e.g. table names), you must use `h()`, which is a shortcut for `htmlspecialchars` that also escapes `"` and `'`. While a templating system would be useful, it would need to support streaming. Adminer prints data immediately to display partial results when a query is slow. -When constructing SQL queries, you must use `q()` for strings and `idf_escape()` for identifiers. Adminer needs full power when constructing queries so using some helper here would be challenging. +When constructing SQL queries, use `q()` for strings and `idf_escape()` for identifiers. Adminer requires full control when constructing queries, making the use of additional helpers challenging. ## Minimalism -Adminer is minimalist in everything - if something doesn't need to be there then it shouldn't be. It's true for UI which I try to keep as uncluttered as possible. E.g. I'm almost never interested in an index name, I'm always interested in columns of that index. So Adminer displays the index name only in `title=""`. The same is true for code - e.g. public visibility is the default so it doesn't need to be explicitly specified. People use `public` to differentiate from the case where they've forgotten to specify the visibility but I don't suffer from this. +Adminer is minimalist in every aspect - if something is unnecessary, it should not be included. This philosophy extends to the UI, which remains as uncluttered as possible. For example, index names are usually irrelevant compared to the columns they reference, so Adminer displays index names only in `title=""`. The same principle applies to the code; for instance, `public` visibility is the default, so it does not need to be explicitly specified. -If something could be implemented in a plugin then I accept it to the core only if it's useful for almost everyone. E.g. [sticky table headers](https://github.com/vrana/adminer/issues/918) are useful for everyone so they have been included. But [dark mode switcher](https://github.com/vrana/adminer/issues/926) would clutter the UI and it's useful only for someone so I've created a plugin for that. +If a feature can be implemented as a plugin, it is only added to the core if it benefits almost everyone. For example, [sticky table headers](https://github.com/vrana/adminer/issues/918) are useful to all users and have been included, whereas a [dark mode switcher](https://github.com/vrana/adminer/issues/926) would clutter the UI and is only useful for some, so it remains a plugin. ## Dependencies -Adminer uses [Git submodules](https://git-scm.com/docs/git-submodule) for dependencies which predates [Composer](https://getcomposer.org/) or other package managers. Submodules are very convenient for developing - e.g. I add some feature to the syntax highlighter, commit this change and then I immediately use it in Adminer. The Adminer's commit just includes the current HEAD of the submodule. I don't need to release a new version for every change, update lock files or do stuff like that. +Adminer uses [Git submodules](https://git-scm.com/docs/git-submodule) for dependencies, predating [Composer](https://getcomposer.org/) and other package managers. Submodules simplify development - for example, I can add a feature to the syntax highlighter, commit the change, and immediately use it in Adminer. Adminer commits simply reference the current HEAD of the submodule, avoiding the need for frequent version releases, lock file updates, or other package management tasks. ## Tests -Adminer doesn't have unit tests but it has quite extensive [end-to-end tests](/tests/). The advantage of these tests is that they verify the correct behavior even in the UI which is otherwise hard to test. They currently run about 10 minutes but it's bearable before releasing a new version. The advantage is that they allow to discover even e.g. JavaScript errors for real use-cases. +Adminer does not include unit tests but has extensive [end-to-end tests](/tests/). These tests verify correct behavior, including UI functionality, which is otherwise difficult to test. The tests take about 10 minutes to run, which is acceptable before a release. They help detect even JavaScript errors in real-world use cases. ## JavaScript -Adminer should work with disabled JavaScript but it's more pleasant with JS enabled. Adminer doesn't use any framework but instead it has simple helpers like `qsa()` which is `document.querySelectorAll()` and then simple functions calling these helpers. These functions used to be bound directly in HTML (``) but enabling strict CSP made this impossible. Adminer now registers these helpers using a short `