Apply suggestions from code review

Co-authored-by: Frédéric Collonval <[email protected]>

Author: navn-r

jupyterlab_git/git.py

@@ -432,19 +432,24 @@ async def status(self, path):
 
         return data
 
-    async def log(self, path, history_count=10, follow_path="."):
+    async def log(self, path, history_count=10, follow_path=None):
         """
         Execute git log command & return the result.
         """
-        is_single_file = follow_path != "."
+        is_single_file = follow_path != None
         cmd = [
             "git",
             "log",
             "--pretty=format:%H%n%an%n%ar%n%s",
             ("-%d" % history_count),
         ]
         if is_single_file:
-            cmd = cmd + ["--numstat", "-z", "--follow", "--", follow_path]
+            cmd = cmd + [
+                "--numstat",
+                "--follow",
+                "--",
+                follow_path,
+            ]
         code, my_output, my_error = await execute(
             cmd,
             cwd=path,
@@ -453,7 +458,10 @@ async def log(self, path, history_count=10, follow_path="."):
             return {"code": code, "command": " ".join(cmd), "message": my_error}
 
         result = []
-        line_array = my_output.replace("\0\0", "\n").splitlines()
+        if is_single_file:
+            # an extra newline get outputted when --numstat is used
+            my_output = my_output.replace("\n\n", "\n")
+        line_array = my_output.splitlines()
         i = 0
         PREVIOUS_COMMIT_OFFSET = 5 if is_single_file else 4
         while i < len(line_array):

jupyterlab_git/handlers.py

@@ -196,7 +196,7 @@ async def post(self, path: str = ""):
 
 class GitLogHandler(GitHandler):
     """
-    Handler for 'git log --pretty=format:%H-%an-%ar-%s --follow --'.
+    Handler for 'git log'.
     Fetches Commit SHA, Author Name, Commit Date & Commit Message.
     """
 
@@ -208,7 +208,7 @@ async def post(self, path: str = ""):
         """
         body = self.get_json_body()
         history_count = body.get("history_count", 25)
-        follow_path = body.get("follow_path", ".")
+        follow_path = body.get("follow_path")
         result = await self.git.log(
             self.url2localpath(path), history_count, follow_path
         )

src/commandsAndMenu.tsx

@@ -898,14 +898,19 @@ export function addCommands(
     label: trans.__('History'),
     caption: trans.__('View the history of this file'),
     execute: args => {
-      const {
-        files: [file]
-      } = args as any as CommandArguments.IGitContextAction;
-      if (file && file.status === 'unmodified') {
+      const { files } = args as any as CommandArguments.IGitContextAction;
+      const file = files[0];
+      if (!file) {
+        return;
+      } else if (file.status === 'unmodified') {
         gitModel.selectedHistoryFile = file;
         shell.activateById('jp-git-sessions');
       }
     },
+    isEnabled: args => {
+      const { files } = args as any as CommandArguments.IGitContextAction;
+      return files.length === 1;
+    },
     icon: historyIcon.bindprops({ stylesheet: 'menuItem' })
   });
 
@@ -1085,10 +1090,9 @@ export function addFileBrowserContextMenu(
           )
       );
 
-      // if looking at a tracked file with no changes,
-      // it has no status, nor any actions available
+      // if looking at a tracked file without any actions available
       // (although `git rm` would be a valid action)
-      if (allCommands.size === 0 && statuses.size === 0) {
+      if (allCommands.size === 0) {
         allCommands.add(ContextCommandIDs.gitNoAction);
       }
 

src/components/HistorySideBar.tsx

@@ -1,11 +1,11 @@
 import { TranslationBundle } from '@jupyterlab/translation';
+import { closeIcon } from '@jupyterlab/ui-components';
 import { CommandRegistry } from '@lumino/commands';
 import * as React from 'react';
 import { CommandArguments } from '../commandsAndMenu';
 import { GitExtension } from '../model';
 import { hiddenButtonStyle } from '../style/ActionButtonStyle';
 import { historySideBarStyle } from '../style/HistorySideBarStyle';
-import { removeIcon } from '../style/icons';
 import { ContextCommandIDs, Git } from '../tokens';
 import { ActionButton } from './ActionButton';
 import { FileItem } from './FileItem';
@@ -112,7 +112,7 @@ export const HistorySideBar: React.FunctionComponent<IHistorySideBarProps> = (
           actions={
             <ActionButton
               className={hiddenButtonStyle}
-              icon={removeIcon}
+              icon={closeIcon}
               title={props.trans.__('Discard file history')}
               onClick={removeSelectedFile}
             />

src/components/PastCommitNode.tsx

@@ -4,7 +4,6 @@ import { CommandRegistry } from '@lumino/commands';
 import * as React from 'react';
 import { classes } from 'typestyle';
 import { GitExtension } from '../model';
-import { diffIcon } from '../style/icons';
 import {
   branchClass,
   branchWrapperClass,
@@ -20,7 +19,6 @@ import {
   workingBranchClass
 } from '../style/PastCommitNode';
 import { Git } from '../tokens';
-import { ActionButton } from './ActionButton';
 
 /**
  * Interface describing component properties.
@@ -107,6 +105,11 @@ export class PastCommitNode extends React.Component<
             ? commitExpandedClass
             : null
         )}
+        title={
+          this.props.children
+            ? this.props.trans.__('View commit details')
+            : this.props.trans.__('View file changes')
+        }
         onClick={this._onCommitClick}
       >
         <div className={commitHeaderClass}>
@@ -119,19 +122,10 @@ export class PastCommitNode extends React.Component<
           <span className={commitHeaderItemClass}>
             {this.props.commit.date}
           </span>
-          {this.props.children ? (
-            this.state.expanded ? (
-              <caretUpIcon.react className={iconButtonClass} tag="span" />
-            ) : (
-              <caretDownIcon.react className={iconButtonClass} tag="span" />
-            )
+          {this.props.children && this.state.expanded ? (
+            <caretUpIcon.react className={iconButtonClass} tag="span" />
           ) : (
-            !!this.props.onOpenDiff && (
-              <ActionButton
-                icon={diffIcon}
-                title={this.props.trans.__('View file changes')}
-              />
-            )
+            <caretDownIcon.react className={iconButtonClass} tag="span" />
           )}
         </div>
         <div className={branchWrapperClass}>{this._renderBranches()}</div>
@@ -198,8 +192,8 @@ export class PastCommitNode extends React.Component<
       this.setState({
         expanded: !this.state.expanded
       });
-    } else if (!!this.props.onOpenDiff) {
-      this.props.onOpenDiff(event);
+    } else {
+      this.props.onOpenDiff?.call(this, event);
     }
   };
 }

src/model.ts

@@ -193,8 +193,10 @@ export class GitExtension implements IGitExtension {
     return this._selectedHistoryFile;
   }
   set selectedHistoryFile(file: Git.IStatusFile | null) {
-    this._selectedHistoryFile = file;
-    this._selectedHistoryFileChanged.emit(file);
+    if (this._selectedHistoryFile !== file) {
+      this._selectedHistoryFile = file;
+      this._selectedHistoryFileChanged.emit(file);
+    }
   }
 
   /**
@@ -722,9 +724,7 @@ export class GitExtension implements IGitExtension {
           'POST',
           {
             history_count: count,
-            follow_path: !!this.selectedHistoryFile
-              ? this.selectedHistoryFile.to
-              : '.'
+            follow_path: this.selectedHistoryFile?.to
           }
         );
       }