Skip to content

Register icons for git stash and nb shelve actions #8373

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.

Sign up for GitHub

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

Merged
mbien merged 1 commit into from
Apr 14, 2025
Merged

Register icons for git stash and nb shelve actions #8373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ide/git/nbproject/project.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@
<compile-dependency/>
<run-dependency/>
</dependency>
<dependency>
<code-name-base>org.netbeans.api.annotations.common</code-name-base>
<build-prerequisite/>
<compile-dependency/>
<run-dependency>
<release-version>1</release-version>
<specification-version>1.56</specification-version>
</run-dependency>
</dependency>
<dependency>
<code-name-base>org.netbeans.api.progress</code-name-base>
<build-prerequisite/>
Expand Down
90 changes: 50 additions & 40 deletions ide/git/src/org/netbeans/modules/git/ui/shelve/ShelveChangesAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.List;
import java.util.ListIterator;
import java.util.Set;
import java.util.concurrent.Callable;
import javax.swing.Action;
import javax.swing.BoxLayout;
import javax.swing.JCheckBox;
Expand All @@ -39,6 +38,7 @@
import javax.swing.JMenuItem;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import org.netbeans.api.annotations.common.StaticResource;
import org.netbeans.libs.git.GitClient.DiffMode;
import org.netbeans.libs.git.GitException;
import org.netbeans.libs.git.GitRevisionInfo;
Expand Down Expand Up @@ -69,18 +69,33 @@
import org.openide.windows.WindowManager;

/**
* Shelve modifications of one or more files in form of a patch file.
*
* @author Ondra Vrabec
*/
@ActionID(id = "org.netbeans.modules.git.ui.shelve.ShelveChangesAction", category = "Git")
@ActionRegistration(displayName = "#CTL_ShelveChanges_Title")
@NbBundle.Messages({
"CTL_ShelveChanges_Title=&Shelve Changes...",
"LBL_ShelveChangesAction_Name=&Shelve Changes..."
"CTL_ShelveChanges_Title=&Shelve selected Changes...",
"LBL_ShelveChangesAction_Name=&Shelve selected Changes..."
})
public class ShelveChangesAction extends SingleRepositoryAction {

private static ShelveChangesActionProvider ACTION_PROVIDER;

// TODO pick/create better icon
@StaticResource
private static final String ICON_RESOURCE = "org/netbeans/modules/git/resources/icons/diff.png"; //NOI18N

public ShelveChangesAction() {
super(ICON_RESOURCE);
}

@Override
protected String iconResource() {
return ICON_RESOURCE;
}

@Override
protected void performAction (File repository, File[] roots, VCSContext context) {
shelve(repository, roots);
Expand All @@ -94,14 +109,13 @@ public void shelve (File repository, File[] roots) {
if (Git.getInstance().getFileStatusCache().listFiles(roots,
FileInformation.STATUS_MODIFIED_HEAD_VS_WORKING).length == 0) {
// no local changes found
EventQueue.invokeLater(new Runnable() {
@Override
public void run () {
JOptionPane.showMessageDialog(WindowManager.getDefault().getMainWindow(),
Bundle.MSG_ShelveAction_noModifications_text(),
Bundle.LBL_ShelveAction_noModifications_title(),
JOptionPane.INFORMATION_MESSAGE);
}
EventQueue.invokeLater(() -> {
JOptionPane.showMessageDialog(
WindowManager.getDefault().getMainWindow(),
Bundle.MSG_ShelveAction_noModifications_text(),
Bundle.LBL_ShelveAction_noModifications_title(),
JOptionPane.INFORMATION_MESSAGE
);
});
return;
}
Expand Down Expand Up @@ -192,33 +206,26 @@ protected void exportPatch (File toFile, File commonParent) throws IOException {
"# {0} - repository name", "MSG_ShelveChanges.progress.reverting=Reverting local changes - {0}"
})
protected void postExportCleanup () {
final Collection<File> notifiedFiles = new HashSet<File>();
final Collection<File> notifiedFiles = new HashSet<>();
if (support.isCanceled()) {
return;
}
try {
GitUtils.runWithoutIndexing(new Callable<Void>() {
@Override
public Void call () throws Exception {
support.setDisplayName(Bundle.MSG_ShelveChanges_progress_reverting(repository.getName()));
// init client
GitClient client = Git.getInstance().getClient(repository);
client.addNotificationListener(new FileListener() {
@Override
public void notifyFile (File file, String relativePathToRoot) {
notifiedFiles.add(file);
}
});
client.addNotificationListener(support.new DefaultFileListener(modifications));

// revert
client.checkout(modifications, doRevertIndex ? GitUtils.HEAD : null,
true, support.getProgressMonitor());
if(doPurge) {
client.clean(modifications, support.getProgressMonitor());
}
return null;
GitUtils.runWithoutIndexing(() -> {
support.setDisplayName(Bundle.MSG_ShelveChanges_progress_reverting(repository.getName()));
// init client
GitClient client = Git.getInstance().getClient(repository);
client.addNotificationListener((FileListener) (file, relativePathToRoot) -> {
notifiedFiles.add(file);
});
client.addNotificationListener(support.new DefaultFileListener(modifications));

// revert
client.checkout(modifications, doRevertIndex ? GitUtils.HEAD : null, true, support.getProgressMonitor());
if(doPurge) {
client.clean(modifications, support.getProgressMonitor());
}
return null;
}, modifications);
} catch (GitException ex) {
GitClientExceptionHandler.notifyException(ex, true);
Expand All @@ -239,8 +246,9 @@ private void startAsync (RequestProcessor rp, final File repository, final File[
support = new ShelveChangesProgressSupport() {
@Override
protected void perform () {
modifications = Git.getInstance().getFileStatusCache().listFiles(roots,
FileInformation.STATUS_MODIFIED_HEAD_VS_WORKING);
modifications = Git.getInstance().getFileStatusCache().listFiles(
roots, FileInformation.STATUS_MODIFIED_HEAD_VS_WORKING
);
// shelve changes builds common root, it must be the repository root folder
// because we use export diff action from the git api
File[] arr = Arrays.copyOf(modifications, modifications.length + 1);
Expand Down Expand Up @@ -274,7 +282,7 @@ public Action getAction () {

@Override
public JComponent[] getUnshelveActions (VCSContext ctx, boolean popup) {
JComponent[] cont = UnshelveMenu.getInstance().getMenu(ctx, popup);
JComponent[] cont = UnstashMenu.getInstance().getMenu(ctx, popup);
if (cont == null) {
cont = super.getUnshelveActions(ctx, popup);
}
Expand All @@ -294,18 +302,20 @@ public void setDisplayName (String displayName) {
}
}

// TODO git unstash in shelve action?

@NbBundle.Messages({
"CTL_UnstashMenu.name=&Git Unstash",
"CTL_UnstashMenu.name.popup=Git Unstash",
"# {0} - stash index", "# {1} - stash name", "CTL_UnstashAction.name={0} - {1}"
})
private static class UnshelveMenu {
private static class UnstashMenu {
Comment on lines +305 to +312
Copy link
Member Author

@mbien mbien Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might indicate that things were copied around. Git unstash should not be in the shelve action.

this lead also to this:

netbeans/ide/versioning.util/src/org/netbeans/modules/versioning/shelve/impl/ShelveChangesMenu.java

Lines 152 to 153 in e2dce56

// actions depending on the central patch storage
ShelveChangesActionProvider actionProvider = ShelveChangesActionsRegistry.getInstance().getActionProvider(vs[0]);

which is not the patch storage, but the git unstash action (unshelve is actually registered a few lines before that)

but this would be for other PRs.


private static UnshelveMenu instance;
private static UnstashMenu instance;

static synchronized UnshelveMenu getInstance () {
static synchronized UnstashMenu getInstance() {
if (instance == null) {
instance = new UnshelveMenu();
instance = new UnstashMenu();
}
return instance;
}
Expand Down
44 changes: 25 additions & 19 deletions ide/git/src/org/netbeans/modules/git/ui/stash/SaveStashAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.Callable;
import org.netbeans.api.annotations.common.StaticResource;
import org.netbeans.libs.git.GitException;
import org.netbeans.modules.git.FileInformation;
import org.netbeans.modules.git.Git;
Expand All @@ -43,6 +43,7 @@
import org.openide.util.NbBundle;

/**
* Git stash action, currently only repo wide.
*
* @author Ondra Vrabec
*/
Expand All @@ -56,7 +57,20 @@
"MSG_SaveStashAction.noModifications=There are no uncommitted changes to stash."
})
public class SaveStashAction extends SingleRepositoryAction {


// TODO pick/create better icon
@StaticResource
private static final String ICON_RESOURCE = "org/netbeans/modules/git/resources/icons/get_clean.png"; //NOI18N
Comment on lines +61 to +63
Copy link
Member Author

@mbien mbien Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eirikbakke having icons for "git stash" and "shelve changes" would be great I think. Possibly the clean icon combined with the diff icon or something like that? (not for NB 26, maybe some day later)

Copy link
Contributor

@eirikbakke eirikbakke Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can look at that the next time I'm doing icon stuff.

Where does the word "shelve" come from? It sounds synonymous with "stash", but it's not an actual git command. Looking at this StackOverflow question, it's an IntelliJ term that seems to confuse users. Maybe it should be renamed in the NetBeans UI?

Git stash via jgit does also only support repo wide "stash push", while shelve works on the selection.

What is the "selection" referred to here?

Copy link
Contributor

@lkishalmi lkishalmi Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shelve is used in Subversion, Perforce and Mercurial (with some extensions). They are used for similar purpose as git stash

Copy link
Member Author

@mbien mbien Apr 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shelve moves changes into patch files and stores those locally, this should work on any VCS. Stash is just git stash. Shelve will only shelve the selected files. Stash will stash the whole repo since I don't think jgit supports file stashing yet (git does for quite a while).

Copy link
Contributor

@eirikbakke eirikbakke Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, maybe I understand now. Perhaps "Stash to Patch File" might be a better name?

Copy link
Contributor

@eirikbakke eirikbakke Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the icon I drew for Shelve, if you think it makes sense:

image

icon_shelve

For stash, we could use ide/git/src/org/netbeans/modules/git/resources/icons/stashes.png, which I have redrawn in SVG as follows:

image

(The latter is part of #8424 )

I can install those two icons in a separate PR later.

Copy link
Contributor

@eirikbakke eirikbakke Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this PR actually add the Stash and Shelve actions to a toolbar somewhere? Which toolbar is it, and how do I access it? (Can't see the context from the very cropped screenshot on top.)

Copy link
Member Author

@mbien mbien Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it only registered the icons for the actions, this is the precondition to add something to the toolbar. User has to do this manually via right click -> customize toolbar.

That is why it wasn't super important for me to pick the right icons. It is just for testing purposes mostly and to get an idea how useful shelve still is. We should update them to the new icons for NB 27 though.

Copy link
Member Author

@mbien mbien Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor

@eirikbakke eirikbakke Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, now I see. Thanks! One annoying thing about the main toolbar is that icons shown there must be prepared in both 16x16 and 24x24 versions. But I can do it for this action. (One would think that the SVG could just be scaled up 150%, but it doesn't look good, as borders end up with a non-standard thickness, and horizontal/vertical lines get pushed off the pixel grid.)


public SaveStashAction() {
super(ICON_RESOURCE);
}

@Override
protected String iconResource() {
return ICON_RESOURCE;
}

@Override
protected void performAction (File repository, File[] roots, VCSContext context) {
saveStash(repository, roots);
Expand All @@ -75,20 +89,16 @@ public void saveStash (final File repository, final File[] roots) {
NotifyDescriptor.DEFAULT_OPTION,
NotifyDescriptor.INFORMATION_MESSAGE,
new Object[]{NotifyDescriptor.OK_OPTION},
NotifyDescriptor.OK_OPTION);
NotifyDescriptor.OK_OPTION
);
DialogDisplayer.getDefault().notifyLater(nd);
return;
}
Mutex.EVENT.readAccess(new Runnable () {

@Override
public void run () {
SaveStash saveStash = new SaveStash(repository, roots, RepositoryInfo.getInstance(repository).getActiveBranch());
if (saveStash.show()) {
start(repository, modifications, saveStash);
}
Mutex.EVENT.readAccess(() -> {
SaveStash saveStash = new SaveStash(repository, roots, RepositoryInfo.getInstance(repository).getActiveBranch());
if (saveStash.show()) {
start(repository, modifications, saveStash);
}

});
}

Expand All @@ -98,13 +108,9 @@ private void start (final File repository, final File[] modifications, final Sav
protected void perform () {
try {
final GitClient client = getClient();
GitUtils.runWithoutIndexing(new Callable<Void>() {

@Override
public Void call () throws Exception {
client.stashSave(saveStash.getMessage(), saveStash.isIncludeUncommitted(), getProgressMonitor());
return null;
}
GitUtils.runWithoutIndexing(() -> {
client.stashSave(saveStash.getMessage(), saveStash.isIncludeUncommitted(), getProgressMonitor());
return null;
}, new File[] { repository });
RepositoryInfo.getInstance(repository).refreshStashes();
} catch (GitException ex) {
Expand Down
Loading