Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ Thumbs.db
Desktop.ini
# Mac crap
.DS_Store
# NetBeans files
# IDE files
/nbproject/
.idea
.vs
.vscode
4 changes: 4 additions & 0 deletions admin/plugins.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@
$data[ $field ] = $plugin[ $value ];
} else {
$data[ $field ] = yourls__('(no info)');
# If it's a URL, set to #
if( in_array( $field, array('uri', 'author_uri') ) ) {
$data[$field] = '#' . $data[$field];
}
Comment on lines +112 to +115
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace error, spaces instead of tabs.

Suggested change
# If it's a URL, set to #
if( in_array( $field, array('uri', 'author_uri') ) ) {
$data[$field] = '#' . $data[$field];
}
# If it's a URL, set to #
if( in_array( $field, array('uri', 'author_uri') ) ) {
$data[$field] = '#' . $data[$field];
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why tabs instead of spaces ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the code around this is using tabs. Mixing them in the same file is not great 😅

The default GH comparison makes this pretty much invisible for projects using 4-space tabs, though. I only noticed because of Refined GitHub adding whitespace symbols to the diff (this is their example of the feature; I can't use the extension on my tablet):

17575287517525951643794844939227

Copy link
Copy Markdown
Member Author

@ozh ozh Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tabs are an old leftover of my early coding style, before Leo told me he hired a hitman to take care of myself if I didn't switch to spaces. Visually it's similar nowadays that all editor can be configured to adjust tab width, and we've been using spaces for years, so I'd rather be consistent with this style and use spaces even if the code around is on tabs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I said I'll pay that hitman to take care of yourself if you mix tabs and spaces in the same file? 🙃

🤣

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative is a gigantic commit to replace all tabs with spaces, which would be a pain for future git blame because it would affect basically every files, and then every commit will be children of this one (we've been there already and it was quite burdensome) (I git blame a lot)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ Sometimes you just have to fix things in the codebase. I get the pain point around blame'ing though.

Really, I've come to believe a lot in the idea of Projectional Editing that just stores a semantic representation of the code that can be formatted for display/editing however the user wants. It stops us from having to even think about formatting conventions at all… but even that would lead us to a Big Commit Touching Everything if we 1) found agreeable tooling and 2) implemented it.

Back to realistic solutions: I feel quite strongly that mixing spaces and tabs in an existing file is the worst outcome, regardless of what the project uses for new code. I really think this patch should just use tabs like the file already is, but I guess you want absolution from @LeoColomb first? 😝

I'll at least dismiss my change request, because you've addressed everything else.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding spaces vs tabs, what I want is simplicity. My editor was (default settings) set to tabs, worked fine, Leo converted me to make it 4 spaces, works fine. When I edit a file, I see if the indentation looks right, but I don't see if it's tabs or spaces. If there's an IDE setting that says "use spaces or use tabs depending on what's around" and it inserts accordingly when I press the Tab key, sure, I don't care. If I have to manually inspect and manually insert Tabs, nah.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were talking about VS Code in another thread recently, so I went and looked that up: https://code.visualstudio.com/docs/editing/codebasics#_autodetection

The magic Google AI tries to tell me that setting is enabled by default, but won't h cite any sources so I can't trust it. Check your settings if you're using VSC. If you're using a different IDE, surely it'll have a similar feature.

Really, any code editor that doesn't auto-detect and match each file's indentation type by default in 2025 doesn't deserve users 😂

(You can merge this any time BTW, if you really don't mind the mixed indents. I'll live 😁)

Copy link
Copy Markdown
Member

@LeoColomb LeoColomb Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tabs are an old leftover of my early coding style, before Leo told me he hired a hitman to take care of myself if I didn't switch to spaces.

True story, I guess I need to end my hitman contract one day or another. Or maybe I should submit an expense on OpenCollective, I'll consider it.

The alternative is a gigantic commit to replace all tabs with spaces, which would be a pain for future git blame because it would affect basically every files.

FAKE NEWS!
No, more seriously, I recently discovered the existence of .git-blame-ignore-revs that might be the solution for all the problems here. See https://akrabat.com/ignoring-revisions-with-git-blame/
And it's supported on all major Git forges, including GitHub.
Not perfect, but could do the trick.

I guess you want absolution from @LeoColomb first? 😝

I'll be honest: YOURLS codebase is already full of tab/space mixture, so it's fine for me either way.
I'm all in for homogeneity, and I'd love to enforce it.
Voilà, you both get my absolution. I should consider doing politics.

In short:

  • Fine for me to merge as is.
  • If both of you agree, a code base cleanup can also be done using the .git-blame-ignore-revs trick. I can do it next month; that will make me happy. 😘

}
unset( $plugin[$value] );
}
Expand Down
25 changes: 8 additions & 17 deletions includes/Database/YDB.php
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<?php

/**
* Aura SQL wrapper for YOURLS that creates the allmighty YDB object.
* Aura SQL wrapper for YOURLS that creates the almighty YDB object.
*
* A fine example of a "class that knows too much" (see https://en.wikipedia.org/wiki/God_object)
*
* Note to plugin authors: you most likely SHOULD NOT use directly methods and properties of this class. Use instead
* function wrappers (eg don't use $ydb->option, or $ydb->set_option(), use yourls_*_options() functions instead).
* function wrappers (e.g. don't use $ydb->option, or $ydb->set_option(), use yourls_*_options() functions instead).
*
* @since 1.7.3
*/
Expand All @@ -33,7 +33,7 @@ class YDB extends ExtendedPdo {
protected $context = '';

/**
* Information related to a short URL keyword (eg timestamp, long URL, ...)
* Information related to a short URL keyword (e.g. timestamp, long URL, ...)
*
* @var array
*
Expand All @@ -53,13 +53,13 @@ class YDB extends ExtendedPdo {
protected $option = [];

/**
* Plugin admin pages informations
* Plugin admin pages information
* @var array
*/
protected $plugin_pages = [];

/**
* Plugin informations
* Plugin information
* @var array
*/
protected $plugins = [];
Expand All @@ -86,7 +86,7 @@ public function __construct($dsn, $user, $pass, $options) {
* Init everything needed
*
* Everything we need to set up is done here in init(), not in the constructor, so even
* when the connection fails (eg config error or DB dead), the constructor has worked
* when the connection fails (e.g. config error or DB dead), the constructor has worked,
* and we have a $ydb object properly instantiated (and for instance yourls_die() can
* correctly die, even if using $ydb methods)
*
Expand Down Expand Up @@ -405,22 +405,13 @@ public function is_installed() {
}

/**
* Return standardized DB version
*
* The regex removes everything that's not a number at the start of the string, or remove anything that's not a number and what
* follows after that.
* 'omgmysql-5.5-ubuntu-4.20' => '5.5'
* 'mysql5.5-ubuntu-4.20' => '5.5'
* '5.5-ubuntu-4.20' => '5.5'
* '5.5-beta2' => '5.5'
* '5.5' => '5.5'
* Return MySQL version
*
* @since 1.7.3
* @return string
*/
public function mysql_version() {
$version = $this->pdo->getAttribute(PDO::ATTR_SERVER_VERSION);
return $version;
return $this->pdo->getAttribute(PDO::ATTR_SERVER_VERSION);
}

}
7 changes: 6 additions & 1 deletion includes/functions-formatting.php
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,12 @@ function yourls_trim_long_string($string, $length = 60, $append = '[...]') {
*
* The regexp searches for the first digits, then a period, then more digits and periods, and discards
* all the rest.
* For instance, 'mysql-5.5-beta' and '5.5-RC1' return '5.5'
* Examples:
* 'omgmysql-5.5-ubuntu-4.20' => '5.5'
* 'mysql5.5-ubuntu-4.20' => '5.5'
* '5.5-ubuntu-4.20' => '5.5'
* '5.5-beta2' => '5.5'
* '5.5' => '5.5'
*
* @since 1.4.1
* @param string $version Version number
Expand Down
2 changes: 1 addition & 1 deletion includes/functions-install.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function yourls_check_database_version() {
}

/**
* Get DB version
* Get DB server version
*
* @since 1.7
* @return string sanitized DB version
Expand Down