From d4aae796c9e54a4be0fb9ce4cf88088452d3ff52 Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Tue, 20 Aug 2013 16:22:14 +0200 Subject: Improve usability of "Create New Tag" dialog * Use hints to have a sane size after opening dialog without having to hardcode minimum size * Make sure advanced content becomes visible when expanded * Instead of changing "enabled" state of text widgets, change "editable". This makes it possible to always select text. * Always enable editing of tag name field, even after existing tag was selected. This allows to select an existing name and then edit it, which is useful for version numbers where only the last part changes. * Use the same key stroke to activate "OK" as in staging view * Add a "clear" icon to the tag name text field * Also allow to replace an existing tag when its name was entered in the tag name text field. Before, this was only possible when selecting it on the right. This also removes some unused code and releases a RevWalk. Bug: 352830 Change-Id: I655ca714667a5ef24a584e41e0c1b3bbe80da949 Signed-off-by: Robin Stocker Signed-off-by: Matthias Sohn --- .../egit/ui/test/team/actions/TagActionTest.java | 2 + .../egit/ui/internal/dialogs/CreateTagDialog.java | 159 +++++++++++---------- 2 files changed, 82 insertions(+), 79 deletions(-) diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/team/actions/TagActionTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/team/actions/TagActionTest.java index e60fe18df4..85fe70928e 100644 --- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/team/actions/TagActionTest.java +++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/team/actions/TagActionTest.java @@ -96,6 +96,8 @@ public class TagActionTest extends LocalRepositoryTestCase { "SomeTag"); assertFalse("Ok should be disabled", tagDialog.bot().button( IDialogConstants.OK_LABEL).isEnabled()); + tagDialog.bot().button(UIText.CreateTagDialog_clearButton) + .click(); tagDialog.bot().textWithLabel(UIText.CreateTagDialog_tagName).setText( "AnotherTag"); assertFalse("Ok should be disabled", tagDialog.bot().button( diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java index 1bf873eec3..e9b13441a0 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/CreateTagDialog.java @@ -1,9 +1,5 @@ /******************************************************************************* - * Copyright (C) 2010, Dariusz Luksza - * Copyright (C) 2011, Mathias Kinzler - * Copyright (C) 2011, Matthias Sohn - * Copyright (C) 2012, IBM Corporation (Markus Keller ) - * + * Copyright (C) 2010, 2013 Dariusz Luksza and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -49,6 +45,7 @@ import org.eclipse.jface.viewers.TableLayout; import org.eclipse.jface.viewers.TableViewer; import org.eclipse.jface.viewers.Viewer; import org.eclipse.jface.viewers.ViewerFilter; +import org.eclipse.jgit.errors.RevisionSyntaxException; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; @@ -69,6 +66,7 @@ import org.eclipse.swt.events.ModifyListener; import org.eclipse.swt.events.SelectionAdapter; import org.eclipse.swt.events.SelectionEvent; import org.eclipse.swt.graphics.Image; +import org.eclipse.swt.graphics.Point; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.widgets.Button; import org.eclipse.swt.widgets.Composite; @@ -103,7 +101,8 @@ public class CreateTagDialog extends TitleAreaDialog { private boolean overwriteTag; - private RevTag tag; + /** Tag object in case an existing annotated tag was entered */ + private RevTag existingTag; private Repository repo; @@ -233,20 +232,10 @@ public class CreateTagDialog extends TitleAreaDialog { return overwriteTag; } - /** - * Data from tag argument will be set in this dialog box. - * - * @param tag - */ - public void setTag(RevTag tag) { - this.tag = tag; - } - @Override protected void configureShell(Shell newShell) { super.configureShell(newShell); newShell.setText(UIText.CreateTagDialog_NewTag); - newShell.setMinimumSize(600, 400); } private String getTitle() { @@ -342,9 +331,6 @@ public class CreateTagDialog extends TitleAreaDialog { createExistingTagsSection(mainForm); mainForm.setWeights(new int[] { 70, 30 }); - if (tag != null) { - setTagImpl(); - } applyDialogFont(parent); return composite; @@ -358,10 +344,8 @@ public class CreateTagDialog extends TitleAreaDialog { tagMessageText.setText(""); //$NON-NLS-1$ if (commitCombo != null) { commitCombo.clearSelection(); - commitCombo.setEnabled(true); } - tagNameText.setEnabled(true); - tagMessageText.setEnabled(true); + tagMessageText.getTextWidget().setEditable(true); overwriteButton.setEnabled(false); overwriteButton.setSelection(false); break; @@ -398,16 +382,22 @@ public class CreateTagDialog extends TitleAreaDialog { label.setLayoutData(data); label.setFont(left.getFont()); - tagNameText = new Text(left, SWT.SINGLE | SWT.BORDER); + tagNameText = new Text(left, SWT.SINGLE | SWT.BORDER | SWT.SEARCH + | SWT.ICON_CANCEL); tagNameText.setLayoutData(new GridData(GridData.GRAB_HORIZONTAL | GridData.HORIZONTAL_ALIGN_FILL)); tagNameText.addModifyListener(new ModifyListener() { public void modifyText(ModifyEvent e) { - String textValue = Pattern.quote(tagNameText.getText()); - tagNamePattern = Pattern.compile(textValue, + String tagNameValue = tagNameText.getText(); + tagNamePattern = Pattern.compile(Pattern.quote(tagNameValue), Pattern.CASE_INSENSITIVE); tagViewer.refresh(); + // Only parse/set tag once (otherwise it would be set twice when + // selecting from the existing tags) + if (existingTag == null + || !tagNameValue.equals(existingTag.getTagName())) + setExistingTagFromText(tagNameValue); validateInput(); } }); @@ -417,24 +407,20 @@ public class CreateTagDialog extends TitleAreaDialog { new Label(left, SWT.WRAP).setText(UIText.CreateTagDialog_tagMessage); + tagMessageText = new SpellcheckableMessageArea(left, tagMessage); - tagMessageText.setLayoutData(GridDataFactory.fillDefaults() - .minSize(50, 50).grab(true, true).create()); + Point size = tagMessageText.getTextWidget().getSize(); + tagMessageText.setLayoutData(GridDataFactory.fillDefaults().hint(size) + .grab(true, true).create()); - // key listener taken from CommitDialog.createDialogArea() allow to - // commit with ctrl-enter + // allow to tag with ctrl-enter tagMessageText.addKeyListener(new KeyAdapter() { - public void keyPressed(KeyEvent arg0) { - if (arg0.keyCode == SWT.CR - && (arg0.stateMask & SWT.CONTROL) > 0) { + public void keyPressed(KeyEvent e) { + if (UIUtils.isSubmitKeyEvent(e)) { Control button = getButton(IDialogConstants.OK_ID); // fire OK action only when button is enabled if (button != null && button.isEnabled()) buttonPressed(IDialogConstants.OK_ID); - } else if (arg0.keyCode == SWT.TAB - && (arg0.stateMask & SWT.SHIFT) == 0) { - arg0.doit = false; - tagMessageText.traverse(SWT.TRAVERSE_TAB_NEXT); } } }); @@ -453,11 +439,6 @@ public class CreateTagDialog extends TitleAreaDialog { overwriteButton.addSelectionListener(new SelectionAdapter() { @Override public void widgetSelected(SelectionEvent e) { - boolean state = overwriteButton.getSelection(); - tagNameText.setEnabled(state); - if (commitCombo != null) - commitCombo.setEnabled(state); - tagMessageText.setEnabled(state); validateInput(); } }); @@ -517,10 +498,11 @@ public class CreateTagDialog extends TitleAreaDialog { commitCombo.add(revCommit); // Set combo selection if a tag is selected - if (tag != null) - commitCombo.setSelectedElement(tag.getObject()); + if (existingTag != null) + commitCombo.setSelectedElement(existingTag.getObject()); } composite.layout(true); + composite.getShell().pack(); } }); } @@ -528,7 +510,7 @@ public class CreateTagDialog extends TitleAreaDialog { private void createExistingTagsSection(Composite parent) { Composite right = new Composite(parent, SWT.NORMAL); right.setLayout(GridLayoutFactory.swtDefaults().create()); - right.setLayoutData(GridLayoutFactory.fillDefaults().create()); + right.setLayoutData(GridDataFactory.fillDefaults().create()); new Label(right, SWT.WRAP).setText(UIText.CreateTagDialog_existingTags); @@ -581,7 +563,7 @@ public class CreateTagDialog extends TitleAreaDialog { } private void validateInput() { - // don't validate if dialog is disposed + // don't do anything if dialog is disposed if (getShell() == null) { return; } @@ -601,62 +583,79 @@ public class CreateTagDialog extends TitleAreaDialog { button.setEnabled(containsTagNameAndMessage || shouldOverwriteTag); } + + boolean existingTagSelected = existingTag != null; + if (existingTagSelected && !overwriteButton.getSelection()) + tagMessageText.getTextWidget().setEditable(false); + else + tagMessageText.getTextWidget().setEditable(true); + + overwriteButton.setEnabled(existingTagSelected); + if (!existingTagSelected) + overwriteButton.setSelection(false); } private void fillTagDialog(ISelection actSelection) { IStructuredSelection selection = (IStructuredSelection) actSelection; Object firstSelected = selection.getFirstElement(); + setExistingTag(firstSelected); + } - if (firstSelected instanceof RevTag) - tag = (RevTag) firstSelected; - else { - setErrorMessage(UIText.CreateTagDialog_LightweightTagMessage); - return; + private void setExistingTagFromText(String tagName) { + RevWalk revWalk = null; + try { + ObjectId tagObjectId = repo.resolve(Constants.R_TAGS + tagName); + if (tagObjectId != null) { + revWalk = new RevWalk(repo); + RevObject tagObject = revWalk.parseAny(tagObjectId); + setExistingTag(tagObject); + return; + } + } catch (IOException e) { + // See below + } catch (RevisionSyntaxException e) { + // See below + } finally { + if (revWalk != null) + revWalk.release(); } + setNoExistingTag(); + } - if (!overwriteButton.isEnabled()) { - String tagMessageValue = tag.getFullMessage(); - // don't enable OK button if we are dealing with un-annotated - // tag because JGit doesn't support them - if (tagMessageValue != null && tagMessageValue.trim().length() != 0) - overwriteButton.setEnabled(true); + private void setNoExistingTag() { + existingTag = null; + } - tagNameText.setEnabled(false); - if (commitCombo != null) - commitCombo.setEnabled(false); - tagMessageText.setEnabled(false); + private void setExistingTag(Object tagObject) { + if (tagObject instanceof RevTag) + existingTag = (RevTag) tagObject; + else { + setNoExistingTag(); + setErrorMessage(UIText.CreateTagDialog_LightweightTagMessage); + return; } - setTagImpl(); - } - - private void setTagImpl() { - tagNameText.setText(tag.getTagName()); + if (!tagNameText.getText().equals(existingTag.getTagName())) + tagNameText.setText(existingTag.getTagName()); if (commitCombo != null) - commitCombo.setSelectedElement(tag.getObject()); + commitCombo.setSelectedElement(existingTag.getObject()); // handle un-annotated tags - String message = tag.getFullMessage(); + String message = existingTag.getFullMessage(); tagMessageText.setText(null != message ? message : ""); //$NON-NLS-1$ } private void getRevCommits(Collection commits) { - RevWalk revWalk = new RevWalk(repo); - revWalk.sort(RevSort.COMMIT_TIME_DESC, true); - revWalk.sort(RevSort.BOUNDARY, true); - + final RevWalk revWalk = new RevWalk(repo); try { + revWalk.sort(RevSort.COMMIT_TIME_DESC, true); + revWalk.sort(RevSort.BOUNDARY, true); AnyObjectId headId = repo.resolve(Constants.HEAD); if (headId != null) revWalk.markStart(revWalk.parseCommit(headId)); - } catch (IOException e) { - Activator.logError(UIText.TagAction_errorWhileGettingRevCommits, e); - setErrorMessage(UIText.TagAction_errorWhileGettingRevCommits); - } - // do the walk to get the commits - RevCommit commit; - long count = 0; - try { + // do the walk to get the commits + long count = 0; + RevCommit commit; while ((commit = revWalk.next()) != null && count < MAX_COMMIT_COUNT) { commits.add(commit); @@ -665,6 +664,8 @@ public class CreateTagDialog extends TitleAreaDialog { } catch (IOException e) { Activator.logError(UIText.TagAction_errorWhileGettingRevCommits, e); setErrorMessage(UIText.TagAction_errorWhileGettingRevCommits); + } finally { + revWalk.release(); } } -- cgit v1.2.3