-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Misc unimportant fixes #3979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Misc unimportant fixes #3979
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
98a0fd9
Misc and small fixes
ozh b3f11e7
Update comments
ozh 91f4d62
Merge branch 'master' into fix-misc
ozh a8af7ec
Update includes/functions-install.php
ozh 1ae69a2
Move comment to the correct function
ozh 6ad9033
Merge branch 'master' into fix-misc
ozh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,8 @@ Thumbs.db | |
| Desktop.ini | ||
| # Mac crap | ||
| .DS_Store | ||
| # NetBeans files | ||
| # IDE files | ||
| /nbproject/ | ||
| .idea | ||
| .vs | ||
| .vscode | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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):
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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? 🙃
🤣
There was a problem hiding this comment.
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 blamebecause 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)There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😁)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
FAKE NEWS!
No, more seriously, I recently discovered the existence of
.git-blame-ignore-revsthat 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'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:
.git-blame-ignore-revstrick. I can do it next month; that will make me happy. 😘