Keep the path as string instead of an array

There is no good reason to keep the path as an array of strings, usually we want to just append a file to the path for file operations as most of the code simply joined the path back to a string. This also makes sure the path is properly formatted as /path/to/thing/ with a leading and trailing /.
cockpit-project · Jul 26, 2024 · f2dd8a3 · f2dd8a3
1 parent 685376f
commit f2dd8a3
Show file tree

Hide file tree

Showing 12 changed files with 77 additions and 57 deletions.
diff --git a/src/app.tsx b/src/app.tsx
@@ -17,7 +17,7 @@
  * along with Cockpit; If not, see <http://www.gnu.org/licenses/>.
  */
 
-import React, { useContext, useEffect, useMemo, useState } from "react";
+import React, { useContext, useEffect, useState } from "react";
 
 import {
     AlertGroup, Alert, AlertVariant, AlertActionCloseButton
@@ -67,6 +67,22 @@ export const FilesContext = React.createContext({
 
 export const useFilesContext = () => useContext(FilesContext);
 
+export const usePath = () => {
+    const { options } = usePageLocation();
+    let currentPath = decodeURIComponent(options.path?.toString() || "/");
+
+    // Our path will always be `/foo/` formatted
+    if (!currentPath.endsWith("/")) {
+        currentPath += "/";
+    }
+
+    if (!currentPath.startsWith("/")) {
+        currentPath = `/${currentPath}`;
+    }
+
+    return currentPath;
+};
+
 export const Application = () => {
     const { options } = usePageLocation();
     const [loading, setLoading] = useState(true);
@@ -79,10 +95,8 @@ export const Application = () => {
     const [alerts, setAlerts] = useState<Alert[]>([]);
     const [cwdInfo, setCwdInfo] = useState<FileInfo | null>(null);
 
-    const currentPath = decodeURIComponent(options.path?.toString() || "");
-    // the function itself is not expensive, but `path` is later used in expensive computation
-    // and changing its reference value on every render causes performance issues
-    const path = useMemo(() => currentPath?.split("/"), [currentPath]);
+    const path = usePath();
+    console.log("path", path);
 
     useEffect(() => {
         cockpit.user().then(user => {
@@ -102,7 +116,7 @@ export const Application = () => {
             setSelected([]);
 
             const client = new FsInfoClient(
-                `/${currentPath}`,
+                path,
                 ["type", "mode", "size", "mtime", "user", "group", "target", "entries", "targets"],
                 { superuser: 'try' }
             );
@@ -126,7 +140,7 @@ export const Application = () => {
                 client.close();
             };
         },
-        [options, currentPath]
+        [options, path]
     );
 
     if (loading)
@@ -159,9 +173,7 @@ export const Application = () => {
                             </Alert>
                         ))}
                     </AlertGroup>
-                    <FilesBreadcrumbs
-                      path={path}
-                    />
+                    <FilesBreadcrumbs path={path} />
                     <PageSection>
                         <Sidebar isPanelRight hasGutter>
                             <SidebarContent>

diff --git a/src/common.ts b/src/common.ts
@@ -53,6 +53,11 @@ export function * map_permissions<T>(func: (value: number, label: string) => T)
 }
 
 export function basename(path: string) {
+    if (path === '/') {
+        return path;
+    }
+
+    path = path.replace(/\/$/, '');
     const elements = path.split('/');
     if (elements.length === 0) {
         return '/';

diff --git a/src/dialogs/permissions.jsx b/src/dialogs/permissions.jsx
@@ -32,7 +32,7 @@ import { etc_group_syntax, etc_passwd_syntax } from 'pam_user_parser';
 import { superuser } from 'superuser';
 
 import { useFilesContext } from '../app';
-import { map_permissions, inode_types } from '../common';
+import { map_permissions, inode_types, basename } from '../common';
 
 const _ = cockpit.gettext;
 
@@ -41,7 +41,7 @@ const EditPermissionsModal = ({ dialogResult, selected, path }) => {
 
     // Nothing selected means we act on the current working directory
     if (!selected) {
-        const directory_name = path[path.length - 1];
+        const directory_name = basename(path);
         selected = { ...cwdInfo, isCwd: true, name: directory_name };
     }
 
@@ -82,7 +82,7 @@ const EditPermissionsModal = ({ dialogResult, selected, path }) => {
         const ownerChanged = owner !== selected.user || group !== selected.group;
 
         try {
-            const directory = selected?.isCwd ? path.join("/") : path.join("/") + "/" + selected.name;
+            const directory = selected?.isCwd ? path : path + selected.name;
             if (permissionChanged)
                 await cockpit.spawn(["chmod", mode.toString(8), directory],
                                     { superuser: "try", err: "message" });

diff --git a/src/dialogs/rename.tsx b/src/dialogs/rename.tsx
@@ -74,7 +74,7 @@ function checkCanOverride(candidate: string, entries: Record<string, FileInfo>,
 
 const RenameItemModal = ({ dialogResult, path, selected } : {
     dialogResult: DialogResult<void>
-    path: string[],
+    path: string,
     selected: FolderFileInfo,
 }) => {
     const { cwdInfo } = useFilesContext();
@@ -84,12 +84,12 @@ const RenameItemModal = ({ dialogResult, path, selected } : {
     const [overrideFileName, setOverrideFileName] = useState(false);
 
     const renameItem = (force = false) => {
-        const newPath = path.join("/") + "/" + name;
+        const newPath = path + name;
         const mvCmd = ["mv", "--no-target-directory"];
         if (force) {
             mvCmd.push("--force");
         }
-        mvCmd.push(path.join("/") + "/" + selected.name, newPath);
+        mvCmd.push(path + selected.name, newPath);
 
         cockpit.spawn(mvCmd, { superuser: "try", err: "message" })
                 .then(() => {
@@ -167,6 +167,6 @@ const RenameItemModal = ({ dialogResult, path, selected } : {
     );
 };
 
-export function show_rename_dialog(dialogs: Dialogs, path: string[], selected: FolderFileInfo) {
+export function show_rename_dialog(dialogs: Dialogs, path: string, selected: FolderFileInfo) {
     dialogs.run(RenameItemModal, { path, selected });
 }
diff --git a/src/files-breadcrumbs.tsx b/src/files-breadcrumbs.tsx
@@ -60,15 +60,14 @@ function useHostname() {
     return hostname;
 }
 
-function BookmarkButton({ path }: { path: string[] }) {
+function BookmarkButton({ path }: { path: string }) {
     const [isOpen, setIsOpen] = React.useState(false);
     const [user, setUser] = React.useState<cockpit.UserInfo | null>(null);
     const [bookmarks, setBookmarks] = React.useState<string[]>([]);
     const [bookmarkHandle, setBookmarkHandle] = React.useState<cockpit.FileHandle<string> | null>(null);
 
     const { addAlert, cwdInfo } = useFilesContext();
 
-    const currentPath = path.join("/") || "/";
     const defaultBookmarks = [];
     if (user?.home)
         defaultBookmarks.push({ name: _("Home"), loc: user?.home }); // TODO: add trash
@@ -122,11 +121,11 @@ function BookmarkButton({ path }: { path: string[] }) {
 
         try {
             await bookmarkHandle.modify((old_content: string) => {
-                if (bookmarks.includes(currentPath)) {
-                    return old_content.split('\n').filter(line => parse_uri(line) !== currentPath)
+                if (bookmarks.includes(path)) {
+                    return old_content.split('\n').filter(line => parse_uri(line) !== path)
                             .join('\n');
                 } else {
-                    const newBoomark = "file://" + path.map(part => encodeURIComponent(part))
+                    const newBoomark = "file://" + path.split('/').map(part => encodeURIComponent(part))
                             .join('/') + "\n";
                     return (old_content || '') + newBoomark;
                 }
@@ -152,8 +151,8 @@ function BookmarkButton({ path }: { path: string[] }) {
         return null;
 
     let actionText = null;
-    if (currentPath !== user.home) {
-        if (bookmarks.includes(currentPath)) {
+    if (path !== user.home) {
+        if (bookmarks.includes(path)) {
             actionText = _("Remove current directory");
         } else if (cwdInfo !== null) {
             actionText = _("Add bookmark");
@@ -206,13 +205,16 @@ function BookmarkButton({ path }: { path: string[] }) {
 }
 
 // eslint-disable-next-line max-len
-export function FilesBreadcrumbs({ path }: { path: string[] }) {
+export function FilesBreadcrumbs({ path }: { path: string }) {
     const [editMode, setEditMode] = React.useState(false);
     const [newPath, setNewPath] = React.useState<string | null>(null);
     const hostname = useHostname();
 
     function navigate(n_parts: number) {
-        cockpit.location.go("/", { path: encodeURIComponent(path.slice(0, n_parts).join("/")) });
+        cockpit.location.go("/", {
+            path: encodeURIComponent(path.split('/').slice(0, n_parts)
+                    .join("/"))
+        });
     }
 
     const handleInputKey = (event: React.KeyboardEvent<HTMLInputElement>) => {
@@ -232,7 +234,7 @@ export function FilesBreadcrumbs({ path }: { path: string[] }) {
 
     const enableEditMode = () => {
         setEditMode(true);
-        setNewPath(path.join("/") || "/");
+        setNewPath(path);
     };
 
     const changePath = () => {
@@ -248,8 +250,12 @@ export function FilesBreadcrumbs({ path }: { path: string[] }) {
         setEditMode(false);
     };
 
-    const fullPath = path.slice(1);
+    const fullPath = path.split('/').slice(1, path.length);
     fullPath.unshift(hostname || "server");
+    // HACK: drop the last /, as the breadcrumb already inserts an artifical /
+    if (fullPath.length > 1) {
+        fullPath.pop();
+    }
 
     return (
         <PageBreadcrumb stickyOnBreakpoint={{ default: "top" }}>
@@ -268,7 +274,7 @@ export function FilesBreadcrumbs({ path }: { path: string[] }) {
                     return (
                         <React.Fragment key={fullPath.slice(0, i).join("/") || "/"}>
                             <Button
-                              isDisabled={i === path.length - 1}
+                              isDisabled={i === fullPath.length - 1}
                               icon={i === 0 ? <HddIcon /> : null}
                               variant="link" onClick={() => { navigate(i + 1) }}
                               className={`breadcrumb-button breadcrumb-${i}`}

diff --git a/src/files-card-body.tsx b/src/files-card-body.tsx
@@ -66,7 +66,7 @@ function compare(sortBy: Sort): (a: FolderFileInfo, b: FolderFileInfo) => number
 }
 
 const ContextMenuItems = ({ path, selected, setSelected, clipboard, setClipboard } : {
-    path: string[],
+    path: string,
     selected: FolderFileInfo[], setSelected: React.Dispatch<React.SetStateAction<FolderFileInfo[]>>,
     clipboard: string[], setClipboard: React.Dispatch<React.SetStateAction<string[]>>,
 }) => {
@@ -115,7 +115,7 @@ export const FilesCardBody = ({
     setCurrentFilter: React.Dispatch<React.SetStateAction<string>>,
     files: FolderFileInfo[],
     isGrid: boolean,
-    path: string[],
+    path: string,
     selected: FolderFileInfo[], setSelected: React.Dispatch<React.SetStateAction<FolderFileInfo[]>>,
     sortBy: Sort, setSortBy: React.Dispatch<React.SetStateAction<Sort>>,
     loadingFiles: boolean,
@@ -150,7 +150,7 @@ export const FilesCardBody = ({
     }
 
     const onDoubleClickNavigate = useCallback((file: FolderFileInfo) => {
-        const newPath = [...path, file.name].join("/");
+        const newPath = path + file.name;
         if (file.to === "dir") {
             cockpit.location.go("/", { path: encodeURIComponent(newPath) });
         }
@@ -272,7 +272,7 @@ export const FilesCardBody = ({
             } else if (e.key === "Enter" && selected.length === 1) {
                 onDoubleClickNavigate(selected[0]);
             } else if (e.key === "Delete" && selected.length !== 0) {
-                confirm_delete(dialogs, path.join("/") + "/", selected, setSelected);
+                confirm_delete(dialogs, path, selected, setSelected);
             }
         };
 

diff --git a/src/files-folder-view.tsx b/src/files-folder-view.tsx
@@ -36,7 +36,7 @@ export const FilesFolderView = ({
     setClipboard,
     setShowHidden,
 }: {
-    path: string[],
+    path: string,
     files: FolderFileInfo[],
     loadingFiles: boolean,
     showHidden: boolean,

diff --git a/src/header.tsx b/src/header.tsx
@@ -111,7 +111,7 @@ export const FilesCardHeader = ({
     isGrid: boolean, setIsGrid: React.Dispatch<React.SetStateAction<boolean>>,
     sortBy: Sort, setSortBy: React.Dispatch<React.SetStateAction<Sort>>
     showHidden: boolean, setShowHidden: React.Dispatch<React.SetStateAction<boolean>>,
-    path: string[],
+    path: string,
 }) => {
     return (
         <CardHeader className="card-actionbar">

diff --git a/src/menu.tsx b/src/menu.tsx
@@ -45,14 +45,13 @@ type MenuItem = { type: "divider" } | {
 };
 
 export function get_menu_items(
-    path: string[],
+    path: string,
     selected: FolderFileInfo[], setSelected: React.Dispatch<React.SetStateAction<FolderFileInfo[]>>,
     clipboard: string[], setClipboard: React.Dispatch<React.SetStateAction<string[]>>,
     cwdInfo: FileInfo | null,
     addAlert: (title: string, variant: AlertVariant, key: string, detail?: string) => void,
     dialogs: Dialogs,
 ) {
-    const currentPath = path.join("/") + "/";
     const menuItems: MenuItem[] = [];
 
     if (selected.length === 0) {
@@ -73,15 +72,15 @@ export function get_menu_items(
                         "cp",
                         "-R",
                         ...clipboard,
-                        currentPath
+                        path
                     ]).catch(err => addAlert(err.message, AlertVariant.danger, `${new Date().getTime()}`));
                 }
             },
             { type: "divider" },
             {
                 id: "create-item",
                 title: _("Create directory"),
-                onClick: () => show_create_directory_dialog(dialogs, currentPath)
+                onClick: () => show_create_directory_dialog(dialogs, path)
             },
             { type: "divider" },
             {
@@ -95,7 +94,7 @@ export function get_menu_items(
             {
                 id: "copy-item",
                 title: _("Copy"),
-                onClick: () => setClipboard([currentPath + selected[0].name]),
+                onClick: () => setClipboard([path + selected[0].name]),
             },
             { type: "divider" },
             {
@@ -113,7 +112,7 @@ export function get_menu_items(
                 id: "delete-item",
                 title: _("Delete"),
                 className: "pf-m-danger",
-                onClick: () => confirm_delete(dialogs, currentPath, selected, setSelected)
+                onClick: () => confirm_delete(dialogs, path, selected, setSelected)
             },
         );
         if (selected[0].type === "reg")
@@ -122,21 +121,21 @@ export function get_menu_items(
                 {
                     id: "download-item",
                     title: _("Download"),
-                    onClick: () => downloadFile(currentPath, selected[0])
+                    onClick: () => downloadFile(path, selected[0])
                 }
             );
     } else if (selected.length > 1) {
         menuItems.push(
             {
                 id: "copy-item",
                 title: _("Copy"),
-                onClick: () => setClipboard(selected.map(s => path.join("/") + "/" + s.name)),
+                onClick: () => setClipboard(selected.map(s => path + s.name)),
             },
             {
                 id: "delete-item",
                 title: _("Delete"),
                 className: "pf-m-danger",
-                onClick: () => confirm_delete(dialogs, currentPath, selected, setSelected)
+                onClick: () => confirm_delete(dialogs, path, selected, setSelected)
             }
         );
     }