nickvergessen
commented
2 weeks ago While I like it for the constructor, I really prefer not to have it on all the other methods. It will be a huge waste of space.
I missed the on_multiline:
Talk diff looks solid
```diff
1) lib/BackgroundJob/CheckHostedSignalingServer.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/BackgroundJob/CheckHostedSignalingServer.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/BackgroundJob/CheckHostedSignalingServer.php
@@ -105,7 +105,8 @@
// only credentials have changed
} elseif ($newStatus === 'active' && (
$oldAccountInfo['signaling']['url'] !== $accountInfo['signaling']['url'] ||
- $oldAccountInfo['signaling']['secret'] !== $accountInfo['signaling']['secret'])
+ $oldAccountInfo['signaling']['secret'] !== $accountInfo['signaling']['secret']
+ )
) {
$this->config->setAppValue('spreed', 'signaling_servers', json_encode([
'servers' => [
----------- end diff -----------
2) lib/TInitialState.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/TInitialState.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/TInitialState.php
@@ -120,7 +120,8 @@
$this->initialState->provideInitialState(
'force_enable_blur_filter',
- $this->serverConfig->getUserValue($user->getUID(), 'theming', 'force_enable_blur_filter', ''));
+ $this->serverConfig->getUserValue($user->getUID(), 'theming', 'force_enable_blur_filter', '')
+ );
$this->initialState->provideInitialState(
'user_group_ids',
----------- end diff -----------
3) lib/Share/RoomShareProvider.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Share/RoomShareProvider.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Share/RoomShareProvider.php
@@ -265,8 +265,10 @@
$entryData = $data;
$entryData['permissions'] = $entryData['f_permissions'];
$entryData['parent'] = $entryData['f_parent'];
- $share->setNodeCacheEntry(Cache::cacheEntryFromData($entryData,
- $this->mimeTypeLoader));
+ $share->setNodeCacheEntry(Cache::cacheEntryFromData(
+ $entryData,
+ $this->mimeTypeLoader
+ ));
}
return $share;
@@ -501,10 +503,24 @@
*/
public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true): array {
$qb = $this->dbConnection->getQueryBuilder();
- $qb->select('s.*',
- 'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
- 'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
- 'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
+ $qb->select(
+ 's.*',
+ 'f.fileid',
+ 'f.path',
+ 'f.permissions AS f_permissions',
+ 'f.storage',
+ 'f.path_hash',
+ 'f.parent AS f_parent',
+ 'f.name',
+ 'f.mimetype',
+ 'f.mimepart',
+ 'f.size',
+ 'f.mtime',
+ 'f.storage_mtime',
+ 'f.encrypted',
+ 'f.unencrypted_size',
+ 'f.etag',
+ 'f.checksum'
)
->from('share', 's')
->andWhere($qb->expr()->orX(
@@ -640,10 +656,24 @@
*/
public function getSharesByIds(array $ids, ?string $recipientId = null): array {
$qb = $this->dbConnection->getQueryBuilder();
- $qb->select('s.*',
- 'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
- 'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
- 'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
+ $qb->select(
+ 's.*',
+ 'f.fileid',
+ 'f.path',
+ 'f.permissions AS f_permissions',
+ 'f.storage',
+ 'f.path_hash',
+ 'f.parent AS f_parent',
+ 'f.name',
+ 'f.mimetype',
+ 'f.mimepart',
+ 'f.size',
+ 'f.mtime',
+ 'f.storage_mtime',
+ 'f.encrypted',
+ 'f.unencrypted_size',
+ 'f.etag',
+ 'f.checksum'
)
->selectAlias('st.id', 'storage_string_id')
->from('share', 's')
@@ -790,10 +820,24 @@
}
$qb = $this->dbConnection->getQueryBuilder();
- $qb->select('s.*',
- 'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash',
- 'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime',
- 'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
+ $qb->select(
+ 's.*',
+ 'f.fileid',
+ 'f.path',
+ 'f.permissions AS f_permissions',
+ 'f.storage',
+ 'f.path_hash',
+ 'f.parent AS f_parent',
+ 'f.name',
+ 'f.mimetype',
+ 'f.mimepart',
+ 'f.size',
+ 'f.mtime',
+ 'f.storage_mtime',
+ 'f.encrypted',
+ 'f.unencrypted_size',
+ 'f.etag',
+ 'f.checksum'
)
->selectAlias('st.id', 'storage_string_id')
->from('share', 's')
----------- end diff -----------
4) lib/Chat/AutoComplete/Sorter.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Chat/AutoComplete/Sorter.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Chat/AutoComplete/Sorter.php
@@ -49,7 +49,8 @@
$type,
array_map(function (array $suggestion) {
return $suggestion['value']['shareWith'];
- }, $byType));
+ }, $byType)
+ );
$search = $context['search'];
----------- end diff -----------
5) lib/Chat/SystemMessage/Listener.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Chat/SystemMessage/Listener.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Chat/SystemMessage/Listener.php
@@ -477,7 +477,9 @@
}
return $this->chatManager->addSystemMessage(
- $room, $actorType, $actorId,
+ $room,
+ $actorType,
+ $actorId,
json_encode(['message' => $message, 'parameters' => $parameters]),
$this->timeFactory->getDateTime(),
$message === 'file_shared',
----------- end diff -----------
6) lib/Settings/Admin/AdminSettings.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Settings/Admin/AdminSettings.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Settings/Admin/AdminSettings.php
@@ -104,10 +104,12 @@
$error = 'binary_permissions';
}
$this->initialState->provideInitialState(
- 'matterbridge_error', $error
+ 'matterbridge_error',
+ $error
);
$this->initialState->provideInitialState(
- 'matterbridge_version', $version
+ 'matterbridge_version',
+ $version
);
$this->initialState->provideInitialState(
@@ -415,7 +417,8 @@
'language' => $userLanguage,
'country' => $guessCountry,
]);
- $this->initialState->provideInitialState('hosted_signaling_server_trial_data',
+ $this->initialState->provideInitialState(
+ 'hosted_signaling_server_trial_data',
json_decode($this->serverConfig->getAppValue('spreed', 'hosted-signaling-server-account', '{}'), true) ?? []
);
$languages = $this->l10nFactory->getLanguages();
----------- end diff -----------
7) lib/Federation/Proxy/TalkV1/Controller/ChatController.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Federation/Proxy/TalkV1/Controller/ChatController.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Federation/Proxy/TalkV1/Controller/ChatController.php
@@ -123,7 +123,8 @@
int $setReadMarker,
int $includeLastKnown,
int $noStatusUpdate,
- int $markNotificationsAsRead): DataResponse {
+ int $markNotificationsAsRead,
+ ): DataResponse {
$cacheKey = sha1(json_encode([$room->getRemoteServer(), $room->getRemoteToken()]));
@@ -165,7 +166,8 @@
);
if ($lookIntoFuture && $setReadMarker) {
- $this->participantService->updateUnreadInfoForProxyParticipant($participant,
+ $this->participantService->updateUnreadInfoForProxyParticipant(
+ $participant,
0,
false,
false,
----------- end diff -----------
8) lib/Manager.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Manager.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Manager.php
@@ -1323,7 +1323,8 @@
// consecutive too fast, so we avoid this as well.
$lastDigit = '0';
for ($i = 0; $i < $entropy; $i++) {
- $lastDigit = $this->secureRandom->generate(1,
+ $lastDigit = $this->secureRandom->generate(
+ 1,
str_replace($lastDigit, '', $chars)
);
$token .= $lastDigit;
----------- end diff -----------
9) tests/integration/features/bootstrap/RecordingTrait.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/tests/integration/features/bootstrap/RecordingTrait.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/tests/integration/features/bootstrap/RecordingTrait.php
@@ -112,7 +112,8 @@
$stdout = tempnam(sys_get_temp_dir(), 'mockserv-stdout-');
// We need to prefix exec to get the correct process http://php.net/manual/ru/function.proc-get-status.php#93382
- $fullCmd = sprintf('exec %s > %s 2>&1',
+ $fullCmd = sprintf(
+ 'exec %s > %s 2>&1',
$cmd,
$stdout
);
----------- end diff -----------
10) lib/Config.php
---------- begin diff ----------
--- /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Config.php
+++ /home/nickv/Nextcloud/31/server/appsbabies/spreed/lib/Config.php
@@ -69,8 +69,10 @@
public function getUserReadPrivacy(string $userId): int {
return match ((int)$this->config->getUserValue(
$userId,
- 'spreed', 'read_status_privacy',
- (string)Participant::PRIVACY_PUBLIC)) {
+ 'spreed',
+ 'read_status_privacy',
+ (string)Participant::PRIVACY_PUBLIC
+ )) {
Participant::PRIVACY_PUBLIC => Participant::PRIVACY_PUBLIC,
default => Participant::PRIVACY_PRIVATE,
};
@@ -82,8 +84,10 @@
public function getUserTypingPrivacy(string $userId): int {
return match ((int)$this->config->getUserValue(
$userId,
- 'spreed', 'typing_privacy',
- (string)Participant::PRIVACY_PUBLIC)) {
+ 'spreed',
+ 'typing_privacy',
+ (string)Participant::PRIVACY_PUBLIC
+ )) {
Participant::PRIVACY_PUBLIC => Participant::PRIVACY_PUBLIC,
default => Participant::PRIVACY_PRIVATE,
};
----------- end diff -----------
```
ChristophWurst
Summary
Change the method_argument_space rule to following:
'method_argument_space' => ['on_multiline' => 'ensure_fully_multiline']Reasons
One of the most common refactoring I see is the following:
I also think it makes sense to have this formatting to reduce the diff if you add another dependency to the constructor, e.g.:
Before:
After: