Skip to content
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.

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

Repositories View: MavenRepo: Add more copy to clipboard action menu entries on right click #5859

Merged

Conversation

chrisrueger
Copy link
Contributor

@chrisrueger chrisrueger commented Oct 31, 2023

This PR adds more Copy to Clipboard actions to a revision of a leaf entry of a Repository item:

image

For example for an entry of com.fasterxml.jackson.core.jackson-databind :

Add GAV to Clipboard

com.fasterxml.jackson.core:jackson-databind:2.13.4 is added to clipboard

Add tooltip to Clipboard

The content of this tooltip when you hover over a leaf-entry is added:

image
com.fasterxml.jackson.core.jackson-databind
General data-binding functionality for Jackson: works on core streaming API
com.fasterxml.jackson.core.jackson-databind
SHA-256: C9FAFF420D9E2C7E1E4711DBEEBEC2506A32C9942027211C5C293D8D87807EB6
Coordinates: com.fasterxml.jackson.core:jackson-databind:2.13.4

is added to the clipboard.

I found this helpful because the tooltip currently cannot be copy pasted and the tooltip contains useful information.

Add Compile Dependencies to Clipboard

com.fasterxml.jackson.core:jackson-annotations:2.13.4
com.fasterxml.jackson.core:jackson-core:2.13.4

is added to the clipboard.
It is basically similar to Add Compile Dependencies

I wanted to add "Runtime Dependencies" too, but always got an empty result. Didn't have time to investigate.
If you find it useful too, it's easy to add too.

Feedback

Consider this PR just a suggestion. My main goal was to have Add GAV to Clipboard because this is handy when researching a dependency on google. The idea for the other two entries just came by playing around with it.
We can remove them if they are too much. Ideally I would like to have a sub-menu "Copy to Clipboard" with those entries, but didn't know how to do that.

I can also add testcases, when we know what to keep and what not.

Signed-off-by: Christoph Rueger <[email protected]>
Signed-off-by: Christoph Rueger <[email protected]>
* github ci build complains, but I don't have this locally. maybe https://bugs.openjdk.org/browse/JDK-8260566 ?

RepoActions.java:195: error: expression type MavenBndRepository is a subtype of pattern type Actionable
			if (repo instanceof Actionable arepo) {

Signed-off-by: Christoph Rueger <[email protected]>
@pkriens
Copy link
Member

pkriens commented Nov 1, 2023

Great! love this work.

I agree it would be nice to have submenus but this would require a bigger change in the support. This code is not allowed to touch Eclipse code in any way because it is part of bndlib. We could make the menu names hierarchical but then we probably want to move the menu builder out of the RepositoriesView and put in some util. I'd say nice to have but there are more interesting things to do right now.

  • Add copy: bsn;version=version ?
  • The copy tooltip & bsn/version could go to the repositories view actions? That would benefit everybody

@kriegfrj
Copy link
Contributor

kriegfrj commented Nov 1, 2023

Great! love this work.

I agree it would be nice to have submenus but this would require a bigger change in the support. This code is not allowed to touch Eclipse code in any way because it is part of bndlib. We could make the menu names hierarchical but then we probably want to move the menu builder out of the RepositoriesView and put in some util. I'd say nice to have but there are more interesting things to do right now.

  • Add copy: bsn;version=version ?
  • The copy tooltip & bsn/version could go to the repositories view actions? That would benefit everybody

A possible alternative to submenus: a single menu item for Copy... which pops up a simple dialogue box to prompt for which details you want to copy?

@chrisrueger
Copy link
Contributor Author

chrisrueger commented Nov 1, 2023

could go to the repositories view actions? That would benefit everybody

Do you mean, move the code for adding the "Copy to clipboard" menu entries e.g. here in bndtools.views.repository.RepositoriesView.createContextMenu() by user manager.add() directly ? @pkriens

image

And remove it from aQute.bnd.repository.maven.provider.RepoActions again?
Just want to clarify if I understand you correctly.

That way we would also have the ability to use Eclipse Code and do more fancy stuff, right?

@chrisrueger
Copy link
Contributor Author

chrisrueger commented Nov 2, 2023

Here is an updated version where I managed to get a "Copy to clipboard" submenu working.

The submenu has a Generic part and a Repo-specific part:

  • Generic part: the submenu no works on all leaf nodes of the Respository view if the entry is instanceof RepositoryBundleVersion which should also work for repositories other than Maven
  • Repo-specific part: Each repository can still contribute own entries to that Submenu if the label startsWith("Copy") (not sure if this hack is reasonable...) see aQute.bnd.repository.maven.provider.RepoActions
image

1. Copy bsn+version

com.fasterxml.jackson.core.jackson-databind;version=2.13.4

2. Copy info

com.fasterxml.jackson.core.jackson-databind
General data-binding functionality for Jackson: works on core streaming API
com.fasterxml.jackson.core.jackson-databind
SHA-256: C9FAFF420D9E2C7E1E4711DBEEBEC2506A32C9942027211C5C293D8D87807EB6
Coordinates: com.fasterxml.jackson.core:jackson-databind:2.13.4

RepositoryBundleVersion [version=2.13.4, bundle=RepositoryBundle [repo=MavenBndRepository [localRepo=/Users/christoph/.m2/repository, storage=Maven Central, inited=true, redeploy=false], bsn=com.fasterxml.jackson.core.jackson-databind]]

Only Maven Repository specific:

3. Copy GAV (only when it is a RepositoryBundleVersion from a MavenRepository )

com.fasterxml.jackson.core:jackson-databind:2.13.4

4. Copy compile Dependencies

com.fasterxml.jackson.core:jackson-annotations:2.13.4
com.fasterxml.jackson.core:jackson-core:2.13.4

* it's just less clutter and both entries served the same purpose which is to have this info for debugging purposes at hand to paste it somewhere.

Signed-off-by: Christoph Rueger <[email protected]>

Just one linebreak

Signed-off-by: Christoph Rueger <[email protected]>
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger chrisrueger marked this pull request as ready for review November 2, 2023 14:41
@pkriens
Copy link
Member

pkriens commented Nov 2, 2023

Yes, you can change the Repositories View to your delight. The only thing that is paramount is that the bndlib code never touches Eclipse code ... Adjacent, the Repositories view must never know anything about the instance classes of the RepositoryPlugin interfaces. I.e. the Repository View is not allowed to cast the RepositoryPlugin to a BndRepositoryPlugin. The interface must fully go through the actions() method. Once the Repositories View starts to know about implementation details of the plugins it will quickly become a mess. i really like the Map<String,Runnable> interface we have now.

I'd be ok to do string matching on the label. We already to checkmarks etc. Making the labels hierarchical would be anice extension. However, if we go that route it would be nice to extract this actions -> menu handling in a separate class so we could potentially use it in other places. The Actionable interface is quite useful in other views.

@chrisrueger
Copy link
Contributor Author

Ok

Making the labels hierarchical would be anice extension. However, if we go that route it would be nice to extract this actions -> menu handling in a separate class so we could potentially use it in other places.

Ok I will give this some thought.
A slash (/) could be used for hierarchy. Do we want to support unlimited hierarchy layers? or just one additional (as in 1 submenu)?

Label Example 1:

  • Copy to Clipboard / Copy info
  • Copy to Clipboard / Copy GAV
  • First layer / Second Layer / Third layer

or

Label Example 2: (double colon as delimiter)

  • Copy to Clipboard :: Copy info
  • Copy to Clipboard :: Copy GAV
  • First layer :: Second Layer :: Third layer

Maybe double colon (::) is less likely to be used in the label text than slash? Thoughts?

remove some method parameters I introduced earlier, which now turned out not to be used anymore.
If it is needed in the future, it can be added again. just don't want to have unused stuff

Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger chrisrueger force-pushed the repository-copy-revision-to-clipboard branch from 2c32b93 to b091bd1 Compare November 2, 2023 20:19
e.g.
Copy to clipboard :: Copy GAV to clipboard
Copy to clipboard :: Copy Compile Dependecies to clipboard

will both be added to a submenu "Copy to clipboard"

Signed-off-by: Christoph Rueger <[email protected]>
HierarchicalLabel<T> can now be used with any type... not just Actions. That way HierarchicalLabel is independent of Eclipse code and could be used in other contexts too. Maybe move more to core.

Signed-off-by: Christoph Rueger <[email protected]>
Not sure if this is good to do this, but I thought at least it gives the implementors and the callers and idea that this String could contain a hierarchy delimiter which can be used for Submenues. If this is not good, we can revert it.

Signed-off-by: Christoph Rueger <[email protected]>
Signed-off-by: Christoph Rueger <[email protected]>
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
Copy link
Contributor Author

@pkriens I think this is ready for review and feedback.
I added support for hierarchical menues using :: as the delimiter.

This

Copy to Clipboard :: Copy bsn;version
Copy to Clipboard :: Copy info
Copy to Clipboard :: Copy GAV
Clear from Cache
Delete from Index

would roughly contribute to this menu and submenu:

image

I also added the Copy to Clipboard :: Copy Info on all levels of RepositoryView now:

  • Repo
  • Bundle
  • BundleRevision

Note: I added a note about the hierarchy delimiter to the Javadoc of Actionable.actions(). If you don't like that , just let me know.

Signed-off-by: Christoph Rueger <[email protected]>
LabelParser takes care of the RegEx parsing

Signed-off-by: Christoph Rueger <[email protected]>
@pkriens
Copy link
Member

pkriens commented Nov 3, 2023

Are we ready to go?

@chrisrueger
Copy link
Contributor Author

Are we ready to go?

If you have no objections, yes.

@chrisrueger
Copy link
Contributor Author

Ah wait... found something. Would fix that first. I give update @pkriens

@pkriens pkriens merged commit 644f558 into bndtools:master Nov 3, 2023
9 checks passed
@pkriens
Copy link
Member

pkriens commented Nov 3, 2023

@chrisrueger sorry, saw your second message too late.

@pkriens
Copy link
Member

pkriens commented Nov 3, 2023

I guess you have to make a new PR :-(

@chrisrueger
Copy link
Contributor Author

I guess you have to make a new PR :-(

Yeah sorry, was a bit of last minute.
Done #5862

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants