Skip to content
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

Proposal: Change method_argument_space to ensure fully multiline #42

Open
susnux opened this issue Nov 12, 2024 · 2 comments
Open

Proposal: Change method_argument_space to ensure fully multiline #42

susnux opened this issue Nov 12, 2024 · 2 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@susnux
Copy link

susnux commented Nov 12, 2024

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:

-public function __construct(LoggerInterface $logger,
+public function __construct(
+	LoggerInterface $logger,
-	IEventDispatcher $dispatcher) {
+ 	IEventDispatcher $dispatcher,
+) {
// something
}

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:

public function __construct(LoggerInterface $logger,
-	IEventDispatcher $dispatcher) {
+ 	IEventDispatcher $dispatcher,
+ 	IURLGenerator $urlGenerator) {
// something
}

After:

public function __construct(
	LoggerInterface $logger,
	IEventDispatcher $dispatcher,
+ 	IURLGenerator $urlGenerator,
) {
// something
}
@susnux susnux added enhancement New feature or request question Further information is requested labels Nov 12, 2024
@nickvergessen
Copy link
Member

nickvergessen commented Nov 13, 2024

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

   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
Copy link
Member

sounds good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants