mattcasters commented on code in PR #8761: URL: https://github.com/apache/hop/pull/8761#discussion_r4192605073
########## plugins/misc/git/src/main/java/org/apache/hop/git/PipelineRevisionDelegate.java: ########## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.git; + +import java.util.List; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.gui.plugin.GuiPlugin; +import org.apache.hop.core.gui.plugin.key.GuiKeyboardShortcut; +import org.apache.hop.core.gui.plugin.key.GuiOsxKeyboardShortcut; +import org.apache.hop.core.gui.plugin.tab.GuiTab; +import org.apache.hop.core.gui.plugin.toolbar.GuiToolbarElement; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.hopgui.file.pipeline.HopGuiPipelineGraph; +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.CTabFolder; +import org.eclipse.swt.custom.CTabItem; + +@GuiPlugin(name = "Pipeline revisions", description = "Revisions of the pipeline") +public class PipelineRevisionDelegate extends BaseRevisionDelegate { + + public static final String GUI_PLUGIN_TOOLBAR_PARENT_ID = "PipelineRevisions-Toolbar"; + public static final String TOOLBAR_ITEM_REFRESH = "PipelineRevisions-Toolbar-10100-Refresh"; + public static final String TOOLBAR_ITEM_SHOW_TEXT_DIFF = + "PipelineRevisions-Toolbar-10200-ShowTextDiff"; + public static final String TOOLBAR_ITEM_SHOW_VISUAL_DIFF = + "PipelineRevisions-Toolbar-10210-ShowVisualDiff"; + + private final HopGuiPipelineGraph pipelineGraph; + + public PipelineRevisionDelegate(HopGui hopGui, HopGuiPipelineGraph pipelineGraph) { + super(hopGui); + this.pipelineGraph = pipelineGraph; + } + + @Override + protected String getToolbarParentId() { + return GUI_PLUGIN_TOOLBAR_PARENT_ID; + } + + @Override + protected String getFilename() { + return pipelineGraph.getPipelineMeta().getFilename(); + } + + @Override + protected boolean hasChanges() { + return pipelineGraph.hasChanged(); + } + + @Override + protected List<String> getSelectionToolbarItemIds() { + return List.of(TOOLBAR_ITEM_SHOW_TEXT_DIFF, TOOLBAR_ITEM_SHOW_VISUAL_DIFF); + } + + @GuiTab( + id = "90000-pipeline-revisions-tab", + parentId = HopGuiPipelineGraph.PIPELINE_GRAPH_TABS, + description = "Pipeline revisions") + public CTabItem createRevisionsTab(CTabFolder tabFolder) { + return super.createRevisionsTab(tabFolder); + } + + /** Refreshes the pipeline revisions with the git history of the pipeline file. */ + @Override + @GuiToolbarElement( + root = GUI_PLUGIN_TOOLBAR_PARENT_ID, + id = TOOLBAR_ITEM_REFRESH, + toolTip = "i18n::System.Button.Refresh", + image = "ui/images/refresh.svg") + @GuiKeyboardShortcut(key = SWT.F5) Review Comment: **[bug]** `@GuiKeyboardShortcut(key = SWT.F5)` (and the same annotation on `WorkflowRevisionDelegate` line 85) is not scoped to the revisions tab. Creating the toolbar calls `registerGuiPluginObject`, and `BaseGuiWidgets.findGuiPluginInstance` then does `HopGuiKeyHandler.addParentObjectToHandle(this)`. The delegate is neither a `Control` nor an `IHopPerspective`, so `HopGuiKeyHandler.isParentInContext` treats it as always in context. The toolbar dispose listener only removes the `GuiRegistry` entry, not the key-handler parent, and `addPluginTabs` constructs a new delegate every time the execution-results panel is shown. Each of those instances stays in `HopGuiKeyHandler.parentObjects` for the session and keeps the `HopGuiPipelineGraph` or `HopGuiWorkflowGraph` reachable after the file is closed. While the explorer perspective is active its own F5 runs first, so the shortcut does not refresh the revisions table even when that table has focus. In a perspective that does not consume F5, the key hits an arbitrary leaked delegate: `refresh()` still walks git history, then `wRevisions.setRedraw` throws because the table was disposed with the panel. That exception is thrown from the `finally` block before `shell.setCursor(null)`, so the shell can stay on the wait cursor. **Suggestion:** Drop the F5 shortcuts, or register the delegate with the tab composite via `addParentObjectToHandle(delegate, composite)` and call `removeParentObjectToHandle` from a dispose listener on that composite. Make `refresh()` return immediately when `wRevisions` is null or disposed, and restore the shell cursor before touching the table. ########## plugins/misc/git/src/main/java/org/apache/hop/git/BaseRevisionDelegate.java: ########## @@ -0,0 +1,349 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.git; + +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import org.apache.commons.vfs2.FileObject; +import org.apache.commons.vfs2.FileSystemException; +import org.apache.hop.core.Const; +import org.apache.hop.core.Props; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.exception.HopFileException; +import org.apache.hop.core.logging.LogChannel; +import org.apache.hop.core.vfs.HopVfs; +import org.apache.hop.git.config.GitConfigSingleton; +import org.apache.hop.git.model.UIGit; +import org.apache.hop.git.model.revision.ObjectRevision; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.ui.core.FormDataBuilder; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.dialog.ErrorDialog; +import org.apache.hop.ui.core.dialog.MessageBox; +import org.apache.hop.ui.core.gui.GuiResource; +import org.apache.hop.ui.core.gui.GuiToolbarWidgets; +import org.apache.hop.ui.core.gui.IToolbarContainer; +import org.apache.hop.ui.core.widget.ColumnInfo; +import org.apache.hop.ui.core.widget.ColumnsResizer; +import org.apache.hop.ui.core.widget.TableView; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.hopgui.ToolbarFacade; +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.CTabFolder; +import org.eclipse.swt.custom.CTabItem; +import org.eclipse.swt.layout.FormLayout; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.TableItem; + +/** Base class sharing the revisions tab between the pipeline and the workflow graphs. */ +public abstract class BaseRevisionDelegate { + + public static final Class<?> PKG = BaseRevisionDelegate.class; // i18n + + protected final HopGui hopGui; + + private GuiToolbarWidgets toolBarWidgets; + private TableView wRevisions; + + /** The revisions shown in the table, in the very same order as its rows. */ + private final List<ObjectRevision> revisions = new ArrayList<>(); + + protected BaseRevisionDelegate(HopGui hopGui) { + super(); + this.hopGui = hopGui; + } + + /** The toolbar root id declared by the subclass, used to create the toolbar widgets. */ + protected abstract String getToolbarParentId(); + + /** The pipeline or workflow filename to get the revisions of, null when it isn't saved yet. */ + protected abstract String getFilename(); + + /** Whether the pipeline or workflow being edited holds changes which aren't written to disk. */ + protected abstract boolean hasChanges(); + + /** The toolbar items which only make sense when a revision is selected in the table. */ + protected abstract List<String> getSelectionToolbarItemIds(); + + /** + * Opens the graphical comparison of two revisions of the file, as a pipeline or as a workflow. + * + * @param relativePath the path of the file, relative to the root of the git repository + * @param commitIdNew the identifier of the most recent version to compare + * @param commitIdOld the identifier of the oldest version to compare + * @throws HopException when the file can't be read or compared + */ + protected abstract void showGraphDiff(String relativePath, String commitIdNew, String commitIdOld) + throws HopException; + + /** Creates the revisions tab in the given tab folder. */ + protected CTabItem createRevisionsTab(CTabFolder tabFolder) { + + // There are no revisions to show if Git plugin is disabled or the project is not a Git + // repository + if (!GitConfigSingleton.getConfig().isEnabled() + || GitGuiPlugin.getInstance().getGit() == null) { + return null; + } + + CTabItem tab = new CTabItem(tabFolder, SWT.NONE); + tab.setFont(GuiResource.getInstance().getFontDefault()); + tab.setImage(GitResource.getInstance().getRevisionImage()); + tab.setText(BaseMessages.getString(PKG, "Revisions.Tab.Name")); + + Composite composite = new Composite(tabFolder, SWT.NONE); + tab.setControl(composite); + composite.setLayout(new FormLayout()); + + // Create toolbar + IToolbarContainer toolBarContainer = + ToolbarFacade.createToolbarContainer(composite, SWT.WRAP | SWT.LEFT | SWT.HORIZONTAL); + toolBarWidgets = new GuiToolbarWidgets(); + toolBarWidgets.registerGuiPluginObject(this); + toolBarWidgets.createToolbarWidgets(toolBarContainer, getToolbarParentId()); + Control toolBar = toolBarContainer.getControl(); + toolBar.setLayoutData(new FormDataBuilder().fullWidth().top().result()); + toolBar.pack(); + PropsUi.setLook(toolBar, Props.WIDGET_STYLE_TOOLBAR); + + // Create the table + ColumnInfo[] revisionColumns = { + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnMessage.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnAuthor.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnDate.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnRevision.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + }; + wRevisions = + new TableView( + hopGui.getVariables(), + composite, + SWT.BORDER | SWT.SINGLE | SWT.FULL_SELECTION, + revisionColumns, + 1, + null, + PropsUi.getInstance()); + wRevisions.setReadonly(true); + wRevisions.setLayoutData(new FormDataBuilder().fullWidth().top(toolBar, 0).bottom().result()); + wRevisions.getTable().addListener(SWT.Resize, new ColumnsResizer(4, 54, 18, 16, 8)); + wRevisions.getTable().addListener(SWT.MouseDoubleClick, event -> showTextDiff()); + wRevisions.getTable().addListener(SWT.Selection, event -> enableToolbarItems()); + PropsUi.setLook(wRevisions); + + this.refresh(); + + // The history can be empty, or not available at all, keep the actions consistent with it + enableToolbarItems(); + + return tab; + } + + /** Refreshes the revisions table with the git history of the current file. */ + public void refresh() { + String fileName = getFilename(); + if (fileName == null) { + return; + } + + // Without a git project there is no revision to show + UIGit git = GitGuiPlugin.getInstance().getGit(); + if (git == null) { + return; + } + + Shell shell = hopGui.getShell(); + + try { + shell.setCursor(shell.getDisplay().getSystemCursor(SWT.CURSOR_WAIT)); + + // The git repository needs a relative path + String relativePath = calculateRelativePath(git.getDirectory(), fileName); + List<ObjectRevision> fileRevisions = git.getRevisions(relativePath); + wRevisions.setRedraw(false); + wRevisions.removeAll(); + revisions.clear(); + for (ObjectRevision revision : fileRevisions) { + if (UIGit.WORKINGTREE.equals(revision.getRevisionId())) { + continue; + } + revisions.add(revision); + + TableItem item = new TableItem(wRevisions.table, SWT.NONE); + item.setText(1, Const.NVL(revision.getComment(), "")); + item.setText(2, Const.NVL(revision.getLogin(), "")); + item.setText(3, getDateString(revision.getCreationDate())); + item.setText(4, git.getShortenedName(revision.getRevisionId())); + } + wRevisions.optimizeTableView(); + + // Preselect the most recent revision so the diff actions always have a subject + if (wRevisions.table.getItemCount() > 0) { + wRevisions.table.setSelection(0); + } + + // A programmatic selection doesn't fire an event, update the actions ourselves + enableToolbarItems(); + } catch (Exception e) { + LogChannel.UI.logError("Error refreshing revisions", e); + } finally { + // Always restore the redraw, otherwise the table stays frozen after an error + wRevisions.setRedraw(true); + shell.setCursor(null); + } + } + + /** + * Opens a side by side text comparison of the selected revision with the file currently being + * edited. The working tree side stays editable, like in the git perspective. + */ + public void showTextDiff() { + try { + RevisionDiff diff = getSelectedRevisionDiff(); + if (diff != null) { + warnAboutUnsavedChanges(); + GitGuiPlugin.getInstance() + .showTextFileDiff(diff.relativePath(), diff.commitIdNew(), diff.commitIdOld()); + } + } catch (Exception e) { + showDiffError("Revisions.ShowTextDiff.Error.Message", getFilename(), e); + } + } + + /** + * Opens a graphical comparison of the selected revision with the file currently being edited, the + * same way the git perspective does for a selected commit. + */ + public void showVisualDiff() { + try { + RevisionDiff diff = getSelectedRevisionDiff(); + if (diff != null) { + warnAboutUnsavedChanges(); + showGraphDiff(diff.relativePath(), diff.commitIdNew(), diff.commitIdOld()); + } + } catch (Exception e) { + showDiffError("Revisions.ShowVisualDiff.Error.Message", getFilename(), e); + } + } + + /** + * Determines what has to be compared, the selected revision with the file currently being edited, + * as it is stored in the working tree. + * + * @return the comparison to show, null when there is nothing to compare + * @throws HopException when the path of the file can't be resolved in the git repository + */ + private RevisionDiff getSelectedRevisionDiff() throws HopException { + String fileName = getFilename(); + if (fileName == null) { + return null; + } + + // Without a git project there is no revision to compare + UIGit git = GitGuiPlugin.getInstance().getGit(); + if (git == null) { + return null; + } + + if (!isRevisionSelected()) { + return null; + } + + try { + String relativePath = calculateRelativePath(git.getDirectory(), fileName); + String revisionId = revisions.get(wRevisions.table.getSelectionIndex()).getRevisionId(); Review Comment: **[bug]** The commit that is diffed is taken from a parallel `revisions` list by `TableView` selection index (`revisions.get(wRevisions.table.getSelectionIndex())`). `TableView` is sortable by default (`sortable = true`) and a header click rebuilds the rows. `TableView.sortTable` keeps `TableItem.getData()` with the visual row, but it does not reorder this list. The cell only shows `git.getShortenedName(...)`, so the full id cannot be recovered from the text. After a sort, the same selection index is a different commit than the one on screen, and the text and visual diff actions compare that other commit. The comment on the list ("in the very same order as its rows") is then false. The git info explorer does not have this bug because it stores the full revision id in the row itself. **Suggestion:** `item.setData(revision)` when the row is created and read that data from the selected `TableItem`. Do not key the commit off the visual index. `TableView.sortTable` already copies `getData()` onto the rebuilt row for this reason. ########## plugins/misc/git/src/main/java/org/apache/hop/git/BaseRevisionDelegate.java: ########## @@ -0,0 +1,349 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.git; + +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import org.apache.commons.vfs2.FileObject; +import org.apache.commons.vfs2.FileSystemException; +import org.apache.hop.core.Const; +import org.apache.hop.core.Props; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.exception.HopFileException; +import org.apache.hop.core.logging.LogChannel; +import org.apache.hop.core.vfs.HopVfs; +import org.apache.hop.git.config.GitConfigSingleton; +import org.apache.hop.git.model.UIGit; +import org.apache.hop.git.model.revision.ObjectRevision; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.ui.core.FormDataBuilder; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.dialog.ErrorDialog; +import org.apache.hop.ui.core.dialog.MessageBox; +import org.apache.hop.ui.core.gui.GuiResource; +import org.apache.hop.ui.core.gui.GuiToolbarWidgets; +import org.apache.hop.ui.core.gui.IToolbarContainer; +import org.apache.hop.ui.core.widget.ColumnInfo; +import org.apache.hop.ui.core.widget.ColumnsResizer; +import org.apache.hop.ui.core.widget.TableView; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.hopgui.ToolbarFacade; +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.CTabFolder; +import org.eclipse.swt.custom.CTabItem; +import org.eclipse.swt.layout.FormLayout; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.TableItem; + +/** Base class sharing the revisions tab between the pipeline and the workflow graphs. */ +public abstract class BaseRevisionDelegate { + + public static final Class<?> PKG = BaseRevisionDelegate.class; // i18n + + protected final HopGui hopGui; + + private GuiToolbarWidgets toolBarWidgets; + private TableView wRevisions; + + /** The revisions shown in the table, in the very same order as its rows. */ + private final List<ObjectRevision> revisions = new ArrayList<>(); + + protected BaseRevisionDelegate(HopGui hopGui) { + super(); + this.hopGui = hopGui; + } + + /** The toolbar root id declared by the subclass, used to create the toolbar widgets. */ + protected abstract String getToolbarParentId(); + + /** The pipeline or workflow filename to get the revisions of, null when it isn't saved yet. */ + protected abstract String getFilename(); + + /** Whether the pipeline or workflow being edited holds changes which aren't written to disk. */ + protected abstract boolean hasChanges(); + + /** The toolbar items which only make sense when a revision is selected in the table. */ + protected abstract List<String> getSelectionToolbarItemIds(); + + /** + * Opens the graphical comparison of two revisions of the file, as a pipeline or as a workflow. + * + * @param relativePath the path of the file, relative to the root of the git repository + * @param commitIdNew the identifier of the most recent version to compare + * @param commitIdOld the identifier of the oldest version to compare + * @throws HopException when the file can't be read or compared + */ + protected abstract void showGraphDiff(String relativePath, String commitIdNew, String commitIdOld) + throws HopException; + + /** Creates the revisions tab in the given tab folder. */ + protected CTabItem createRevisionsTab(CTabFolder tabFolder) { + + // There are no revisions to show if Git plugin is disabled or the project is not a Git + // repository + if (!GitConfigSingleton.getConfig().isEnabled() + || GitGuiPlugin.getInstance().getGit() == null) { + return null; + } + + CTabItem tab = new CTabItem(tabFolder, SWT.NONE); + tab.setFont(GuiResource.getInstance().getFontDefault()); + tab.setImage(GitResource.getInstance().getRevisionImage()); + tab.setText(BaseMessages.getString(PKG, "Revisions.Tab.Name")); + + Composite composite = new Composite(tabFolder, SWT.NONE); + tab.setControl(composite); + composite.setLayout(new FormLayout()); + + // Create toolbar + IToolbarContainer toolBarContainer = + ToolbarFacade.createToolbarContainer(composite, SWT.WRAP | SWT.LEFT | SWT.HORIZONTAL); + toolBarWidgets = new GuiToolbarWidgets(); + toolBarWidgets.registerGuiPluginObject(this); + toolBarWidgets.createToolbarWidgets(toolBarContainer, getToolbarParentId()); + Control toolBar = toolBarContainer.getControl(); + toolBar.setLayoutData(new FormDataBuilder().fullWidth().top().result()); + toolBar.pack(); + PropsUi.setLook(toolBar, Props.WIDGET_STYLE_TOOLBAR); + + // Create the table + ColumnInfo[] revisionColumns = { + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnMessage.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnAuthor.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnDate.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnRevision.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + }; + wRevisions = + new TableView( + hopGui.getVariables(), + composite, + SWT.BORDER | SWT.SINGLE | SWT.FULL_SELECTION, + revisionColumns, + 1, + null, + PropsUi.getInstance()); + wRevisions.setReadonly(true); + wRevisions.setLayoutData(new FormDataBuilder().fullWidth().top(toolBar, 0).bottom().result()); + wRevisions.getTable().addListener(SWT.Resize, new ColumnsResizer(4, 54, 18, 16, 8)); + wRevisions.getTable().addListener(SWT.MouseDoubleClick, event -> showTextDiff()); + wRevisions.getTable().addListener(SWT.Selection, event -> enableToolbarItems()); + PropsUi.setLook(wRevisions); + + this.refresh(); + + // The history can be empty, or not available at all, keep the actions consistent with it + enableToolbarItems(); + + return tab; + } + + /** Refreshes the revisions table with the git history of the current file. */ + public void refresh() { + String fileName = getFilename(); + if (fileName == null) { + return; + } + + // Without a git project there is no revision to show + UIGit git = GitGuiPlugin.getInstance().getGit(); + if (git == null) { + return; + } + + Shell shell = hopGui.getShell(); + + try { + shell.setCursor(shell.getDisplay().getSystemCursor(SWT.CURSOR_WAIT)); + + // The git repository needs a relative path + String relativePath = calculateRelativePath(git.getDirectory(), fileName); + List<ObjectRevision> fileRevisions = git.getRevisions(relativePath); Review Comment: **[suggestion]** `createRevisionsTab` calls `refresh()` on the UI thread, and `UIGit.getRevisions` runs `git status` (and, when the tree is dirty, staged and unstaged lookups) plus `git log` for the path. `addAllTabs` invokes this while opening the execution-results panel, before the panel is shown. Log lines posted with `asyncExec` wait behind that walk, so "show execution results" during a run freezes the shell on a repository that has a long history. The wait cursor does not make the log tab usable. **Suggestion:** Fill the table asynchronously and leave the log, grid, and check tabs responsive. Show an error in the tab instead of only `LogChannel.UI.logError` when the path cannot be resolved, and skip the walk when the file is not inside `git.getDirectory()`. ########## plugins/misc/git/src/main/java/org/apache/hop/git/BaseRevisionDelegate.java: ########## @@ -0,0 +1,349 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.git; + +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import org.apache.commons.vfs2.FileObject; +import org.apache.commons.vfs2.FileSystemException; +import org.apache.hop.core.Const; +import org.apache.hop.core.Props; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.exception.HopFileException; +import org.apache.hop.core.logging.LogChannel; +import org.apache.hop.core.vfs.HopVfs; +import org.apache.hop.git.config.GitConfigSingleton; +import org.apache.hop.git.model.UIGit; +import org.apache.hop.git.model.revision.ObjectRevision; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.ui.core.FormDataBuilder; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.dialog.ErrorDialog; +import org.apache.hop.ui.core.dialog.MessageBox; +import org.apache.hop.ui.core.gui.GuiResource; +import org.apache.hop.ui.core.gui.GuiToolbarWidgets; +import org.apache.hop.ui.core.gui.IToolbarContainer; +import org.apache.hop.ui.core.widget.ColumnInfo; +import org.apache.hop.ui.core.widget.ColumnsResizer; +import org.apache.hop.ui.core.widget.TableView; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.hopgui.ToolbarFacade; +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.CTabFolder; +import org.eclipse.swt.custom.CTabItem; +import org.eclipse.swt.layout.FormLayout; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.TableItem; + +/** Base class sharing the revisions tab between the pipeline and the workflow graphs. */ +public abstract class BaseRevisionDelegate { + + public static final Class<?> PKG = BaseRevisionDelegate.class; // i18n + + protected final HopGui hopGui; + + private GuiToolbarWidgets toolBarWidgets; + private TableView wRevisions; + + /** The revisions shown in the table, in the very same order as its rows. */ + private final List<ObjectRevision> revisions = new ArrayList<>(); + + protected BaseRevisionDelegate(HopGui hopGui) { + super(); + this.hopGui = hopGui; + } + + /** The toolbar root id declared by the subclass, used to create the toolbar widgets. */ + protected abstract String getToolbarParentId(); + + /** The pipeline or workflow filename to get the revisions of, null when it isn't saved yet. */ + protected abstract String getFilename(); + + /** Whether the pipeline or workflow being edited holds changes which aren't written to disk. */ + protected abstract boolean hasChanges(); + + /** The toolbar items which only make sense when a revision is selected in the table. */ + protected abstract List<String> getSelectionToolbarItemIds(); + + /** + * Opens the graphical comparison of two revisions of the file, as a pipeline or as a workflow. + * + * @param relativePath the path of the file, relative to the root of the git repository + * @param commitIdNew the identifier of the most recent version to compare + * @param commitIdOld the identifier of the oldest version to compare + * @throws HopException when the file can't be read or compared + */ + protected abstract void showGraphDiff(String relativePath, String commitIdNew, String commitIdOld) + throws HopException; + + /** Creates the revisions tab in the given tab folder. */ + protected CTabItem createRevisionsTab(CTabFolder tabFolder) { + + // There are no revisions to show if Git plugin is disabled or the project is not a Git + // repository + if (!GitConfigSingleton.getConfig().isEnabled() + || GitGuiPlugin.getInstance().getGit() == null) { + return null; + } + + CTabItem tab = new CTabItem(tabFolder, SWT.NONE); + tab.setFont(GuiResource.getInstance().getFontDefault()); + tab.setImage(GitResource.getInstance().getRevisionImage()); + tab.setText(BaseMessages.getString(PKG, "Revisions.Tab.Name")); + + Composite composite = new Composite(tabFolder, SWT.NONE); + tab.setControl(composite); + composite.setLayout(new FormLayout()); + + // Create toolbar Review Comment: **[suggestion]** Several comments restate the next statement instead of a constraint: "Create toolbar", "Create the table", "Without a git project there is no revision to show", "The git repository needs a relative path", and the one-line javadoc on `refresh`, `showTextDiff`, and `showVisualDiff`. `PipelineRevisionDelegate` and `WorkflowRevisionDelegate` repeat that with "{@inheritDoc}" and javadoc that only says the override refreshes or compares. The comments that explain a real SWT or git constraint (programmatic selection does not fire an event, redraw must be restored, the diff is against the last saved file) are the ones worth keeping. **Suggestion:** Delete the comments that narrate the code. Keep the ones that say why an extra call or a warning exists. ########## plugins/misc/git/src/main/java/org/apache/hop/git/BaseRevisionDelegate.java: ########## @@ -0,0 +1,349 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.git; + +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import org.apache.commons.vfs2.FileObject; +import org.apache.commons.vfs2.FileSystemException; +import org.apache.hop.core.Const; +import org.apache.hop.core.Props; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.exception.HopFileException; +import org.apache.hop.core.logging.LogChannel; +import org.apache.hop.core.vfs.HopVfs; +import org.apache.hop.git.config.GitConfigSingleton; +import org.apache.hop.git.model.UIGit; +import org.apache.hop.git.model.revision.ObjectRevision; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.ui.core.FormDataBuilder; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.dialog.ErrorDialog; +import org.apache.hop.ui.core.dialog.MessageBox; +import org.apache.hop.ui.core.gui.GuiResource; +import org.apache.hop.ui.core.gui.GuiToolbarWidgets; +import org.apache.hop.ui.core.gui.IToolbarContainer; +import org.apache.hop.ui.core.widget.ColumnInfo; +import org.apache.hop.ui.core.widget.ColumnsResizer; +import org.apache.hop.ui.core.widget.TableView; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.hopgui.ToolbarFacade; +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.CTabFolder; +import org.eclipse.swt.custom.CTabItem; +import org.eclipse.swt.layout.FormLayout; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.TableItem; + +/** Base class sharing the revisions tab between the pipeline and the workflow graphs. */ +public abstract class BaseRevisionDelegate { + + public static final Class<?> PKG = BaseRevisionDelegate.class; // i18n + + protected final HopGui hopGui; + + private GuiToolbarWidgets toolBarWidgets; + private TableView wRevisions; + + /** The revisions shown in the table, in the very same order as its rows. */ + private final List<ObjectRevision> revisions = new ArrayList<>(); + + protected BaseRevisionDelegate(HopGui hopGui) { + super(); + this.hopGui = hopGui; + } + + /** The toolbar root id declared by the subclass, used to create the toolbar widgets. */ + protected abstract String getToolbarParentId(); + + /** The pipeline or workflow filename to get the revisions of, null when it isn't saved yet. */ + protected abstract String getFilename(); + + /** Whether the pipeline or workflow being edited holds changes which aren't written to disk. */ + protected abstract boolean hasChanges(); + + /** The toolbar items which only make sense when a revision is selected in the table. */ + protected abstract List<String> getSelectionToolbarItemIds(); + + /** + * Opens the graphical comparison of two revisions of the file, as a pipeline or as a workflow. + * + * @param relativePath the path of the file, relative to the root of the git repository + * @param commitIdNew the identifier of the most recent version to compare + * @param commitIdOld the identifier of the oldest version to compare + * @throws HopException when the file can't be read or compared + */ + protected abstract void showGraphDiff(String relativePath, String commitIdNew, String commitIdOld) + throws HopException; + + /** Creates the revisions tab in the given tab folder. */ + protected CTabItem createRevisionsTab(CTabFolder tabFolder) { + + // There are no revisions to show if Git plugin is disabled or the project is not a Git + // repository + if (!GitConfigSingleton.getConfig().isEnabled() + || GitGuiPlugin.getInstance().getGit() == null) { + return null; + } + + CTabItem tab = new CTabItem(tabFolder, SWT.NONE); + tab.setFont(GuiResource.getInstance().getFontDefault()); + tab.setImage(GitResource.getInstance().getRevisionImage()); + tab.setText(BaseMessages.getString(PKG, "Revisions.Tab.Name")); + + Composite composite = new Composite(tabFolder, SWT.NONE); + tab.setControl(composite); + composite.setLayout(new FormLayout()); + + // Create toolbar + IToolbarContainer toolBarContainer = + ToolbarFacade.createToolbarContainer(composite, SWT.WRAP | SWT.LEFT | SWT.HORIZONTAL); + toolBarWidgets = new GuiToolbarWidgets(); + toolBarWidgets.registerGuiPluginObject(this); + toolBarWidgets.createToolbarWidgets(toolBarContainer, getToolbarParentId()); + Control toolBar = toolBarContainer.getControl(); + toolBar.setLayoutData(new FormDataBuilder().fullWidth().top().result()); + toolBar.pack(); + PropsUi.setLook(toolBar, Props.WIDGET_STYLE_TOOLBAR); + + // Create the table + ColumnInfo[] revisionColumns = { + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnMessage.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnAuthor.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnDate.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + new ColumnInfo( + BaseMessages.getString(PKG, "Revisions.ColumnRevision.Label"), + ColumnInfo.COLUMN_TYPE_TEXT, + false, + true), + }; + wRevisions = + new TableView( + hopGui.getVariables(), + composite, + SWT.BORDER | SWT.SINGLE | SWT.FULL_SELECTION, + revisionColumns, + 1, + null, + PropsUi.getInstance()); + wRevisions.setReadonly(true); + wRevisions.setLayoutData(new FormDataBuilder().fullWidth().top(toolBar, 0).bottom().result()); + wRevisions.getTable().addListener(SWT.Resize, new ColumnsResizer(4, 54, 18, 16, 8)); + wRevisions.getTable().addListener(SWT.MouseDoubleClick, event -> showTextDiff()); + wRevisions.getTable().addListener(SWT.Selection, event -> enableToolbarItems()); + PropsUi.setLook(wRevisions); + + this.refresh(); + + // The history can be empty, or not available at all, keep the actions consistent with it + enableToolbarItems(); + + return tab; + } + + /** Refreshes the revisions table with the git history of the current file. */ + public void refresh() { + String fileName = getFilename(); + if (fileName == null) { + return; + } + + // Without a git project there is no revision to show + UIGit git = GitGuiPlugin.getInstance().getGit(); + if (git == null) { + return; + } + + Shell shell = hopGui.getShell(); + + try { + shell.setCursor(shell.getDisplay().getSystemCursor(SWT.CURSOR_WAIT)); + + // The git repository needs a relative path + String relativePath = calculateRelativePath(git.getDirectory(), fileName); + List<ObjectRevision> fileRevisions = git.getRevisions(relativePath); + wRevisions.setRedraw(false); + wRevisions.removeAll(); + revisions.clear(); + for (ObjectRevision revision : fileRevisions) { + if (UIGit.WORKINGTREE.equals(revision.getRevisionId())) { + continue; + } + revisions.add(revision); + + TableItem item = new TableItem(wRevisions.table, SWT.NONE); + item.setText(1, Const.NVL(revision.getComment(), "")); + item.setText(2, Const.NVL(revision.getLogin(), "")); + item.setText(3, getDateString(revision.getCreationDate())); + item.setText(4, git.getShortenedName(revision.getRevisionId())); + } + wRevisions.optimizeTableView(); + + // Preselect the most recent revision so the diff actions always have a subject + if (wRevisions.table.getItemCount() > 0) { + wRevisions.table.setSelection(0); + } + + // A programmatic selection doesn't fire an event, update the actions ourselves + enableToolbarItems(); + } catch (Exception e) { + LogChannel.UI.logError("Error refreshing revisions", e); + } finally { + // Always restore the redraw, otherwise the table stays frozen after an error + wRevisions.setRedraw(true); + shell.setCursor(null); + } + } + + /** + * Opens a side by side text comparison of the selected revision with the file currently being + * edited. The working tree side stays editable, like in the git perspective. + */ + public void showTextDiff() { + try { + RevisionDiff diff = getSelectedRevisionDiff(); + if (diff != null) { + warnAboutUnsavedChanges(); + GitGuiPlugin.getInstance() + .showTextFileDiff(diff.relativePath(), diff.commitIdNew(), diff.commitIdOld()); + } + } catch (Exception e) { + showDiffError("Revisions.ShowTextDiff.Error.Message", getFilename(), e); + } + } + + /** + * Opens a graphical comparison of the selected revision with the file currently being edited, the + * same way the git perspective does for a selected commit. + */ + public void showVisualDiff() { + try { + RevisionDiff diff = getSelectedRevisionDiff(); + if (diff != null) { + warnAboutUnsavedChanges(); + showGraphDiff(diff.relativePath(), diff.commitIdNew(), diff.commitIdOld()); + } + } catch (Exception e) { + showDiffError("Revisions.ShowVisualDiff.Error.Message", getFilename(), e); + } + } + + /** + * Determines what has to be compared, the selected revision with the file currently being edited, + * as it is stored in the working tree. + * + * @return the comparison to show, null when there is nothing to compare + * @throws HopException when the path of the file can't be resolved in the git repository + */ + private RevisionDiff getSelectedRevisionDiff() throws HopException { + String fileName = getFilename(); Review Comment: **[bug]** History is loaded only from `createRevisionsTab` and the Refresh button. The tab is not rebuilt when the open file's name changes (`HopGuiPipelineGraph.saveAs` / `HopGuiWorkflowGraph.saveAs`, and an explorer rename that calls `setFilename` on the open handler). `checkTabExist` leaves the existing tab in place. `getSelectedRevisionDiff` then resolves the new filename but still uses the commit id stored for the previous path. Commit ids are repository-wide, so the diff is of the new file at an unrelated commit (or a "file not found" error) while the table still lists the old file's messages. The same gap leaves the table empty after the first save of a pipeline or workflow that already had the results panel open. **Suggestion:** Refresh when the graph filename changes (save of a new file, Save As, and rename). Until that runs, do not diff: either clear the table when `getFilename()` no longer matches the path the rows were loaded for, or keep that path next to the revision list and refuse the diff when they differ. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
