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.

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

Jump to bottom

Template fragments: show Author column to make it clear when you pull in content from non-bndtools / 3rd-party #6285

Merged
merged 15 commits into from
Oct 2, 2024
Merged

Template fragments: show Author column to make it clear when you pull in content from non-bndtools / 3rd-party #6285

merged 15 commits into from
Oct 2, 2024

Conversation

chrisrueger
Copy link
Contributor

@chrisrueger chrisrueger commented Sep 23, 2024 •
edited
Loading

Closes #6286

This allows a user to see where a template fragment is coming from. E.g. if the github organisation of the fragment is not "bndtools" we show "Org-name (3rd Party)"
A double click on the entry opens the Github Repo of the template in the browser (e.g. https://github.com/mnlipp/de.mnl.osgi/tree/HEAD/de.mnl.osgi.bnd.repository/workspace-template).

image

A template definition like this is from an external 3rd-party author.

mnlipp/de.mnl.osgi/de.mnl.osgi.bnd.repository/workspace-template#master; \
        name=IndexedMavenRepository; \
        description="Provide an OSGi repository derived from a subset of one or more maven repositories";\

Confirmation popup

If you have selected at least one 3rd-party template, a confirmation popup shows up. We do this for security reason, because you are downloading content from a potentially untrusted source.

image

But this allows to go with the practise that 3rd-party authors can point to their master / main branch, which has the practical effect, that users always get the latest template, in case the 3rd-party author makes updates.

@chrisrueger chrisrueger marked this pull request as draft September 23, 2024 13:59
@chrisrueger chrisrueger mentioned this pull request Sep 23, 2024
Closed
@chrisrueger chrisrueger changed the title show Author column for Template fragments Template fragments: show Author column and let user chose an optional snapshot version Sep 25, 2024
@chrisrueger chrisrueger marked this pull request as ready for review September 25, 2024 12:58
@chrisrueger chrisrueger mentioned this pull request Sep 25, 2024
Open
@chrisrueger chrisrueger changed the title Template fragments: show Author column and let user chose an optional snapshot version Template fragments: show Author column to make it clear when you pull in content from non-bndtools / 3rd-party Sep 27, 2024
@chrisrueger
Copy link
Contributor Author

As discussed in the call with @pkriens and @juergen-albert I will revert / discard the "snapshot" attribute idea.
Instead I will do:

  • keep the Author column
  • show a green checkmark icon if organization is bndtools (Official)
  • show a yellow exclamation mark icon if it is a 3rd-party (non bndtools org)
  • maybe print a warning / confirmation dialog if you have checked a template from a 3rd-party so that the user has to acknowledge that again

Based on this we decided to also allow Template authors to point to their master branch (@mnlipp )

I will work on that the next days and update the PR accordingly.

@chrisrueger
show Author column for Template fragments
621a12f 
This allows a user to see where a template fragment is coming from.
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
extend test for commit SHA
69cea6f 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
add docs for commit SHA
762e5a8 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
fix master repoUrl (without SHA)
9b0c0d1 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
add opt. snapshot to TemplateInfo
b4dc9e3 
experimental as described here:
bndtools#6286 (comment)

example:
bndtools/workspace-templates/gradle#567648ff425693b27b191bd38ace7c9c10539c2d;name=b;description=B;snapshot=bndtools/workspace-templates/gradle#master

This defines a default TemplateID to a specific commit SHA and a snapshot to the master (HEAD branch)

Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger chrisrueger force-pushed the extend-workspace-template-wizzard branch from c4456c9 to b4dc9e3 Compare September 27, 2024 16:32
@chrisrueger
discard snapshot idea
2d1c096 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
confirm dialog if 3rdparty template selected
3637df0 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
show green / warn icons
9b92df1 
green = official bndtools org
yellow warning icon = 3rdParty org

Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
javadoc
0cfa975 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
update docs template fragments
5ce6573 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
remove left over
ccae574 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
Copy link
Contributor Author

chrisrueger commented Sep 27, 2024 •
edited
Loading

@pkriens @mnlipp @juergen-albert I have pushed and updated the PR description with the latest state, based on our discussion earlier. Also adjusted the docs.

@mnlipp are you ok with your repo appearing in the screenshot?

Feedback welcome (e.g. for the Confirmation popup)

@chrisrueger
user master as default in URL again
b6412fe 
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
fix npe
39d468f 
Not sure how I got it but just saw this stacktrace in the Eclipse Error Log

java.lang.NullPointerException: Cannot invoke "String.isBlank()" because "selectedPath" is null
	at bndtools.wizards.newworkspace.NewWorkspaceWizard$NewWorkspaceWizardPage.addTemplate(NewWorkspaceWizard.java:318)
	at bndtools.util.ui.UI$Target.lambda$1(UI.java:273)
	at bndtools.util.ui.UI$3.lambda$0(UI.java:439)
	at bndtools.util.ui.UI$6.widgetSelected(UI.java:508)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:265)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4660)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1622)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1645)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1630)
	at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1392)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4427)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4003)
	at org.eclipse.jface.window.Window.runEventLoop(Window.java:823)
	at org.eclipse.jface.window.Window.open(Window.java:799)
	at org.eclipse.ui.internal.actions.NewWizardShortcutAction.run(NewWizardShortcutAction.java:130)
	at org.eclipse.jface.action.Action.runWithEvent(Action.java:474)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:580)
	at org.eclipse.jface.action.ActionContributionItem.lambda$4(ActionContributionItem.java:414)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4660)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1622)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1645)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1630)
	at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1392)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4427)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4003)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1155)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1046)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:643)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:550)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:171)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.bndtools.applaunch.BndApplicationLauncher.lambda$0(BndApplicationLauncher.java:34)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
	at aQute.launcher.Launcher.handleMainCallable(Launcher.java:464)
	at aQute.launcher.Launcher.launch(Launcher.java:437)
	at aQute.launcher.Launcher.run(Launcher.java:185)
	at aQute.launcher.Launcher.main(Launcher.java:161)
	at aQute.launcher.pre.EmbeddedLauncher.executeWithRunPath(EmbeddedLauncher.java:169)
	at aQute.launcher.pre.EmbeddedLauncher.findAndExecute(EmbeddedLauncher.java:134)
	at aQute.launcher.pre.EmbeddedLauncher.main(EmbeddedLauncher.java:51)

Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
show example url
3e12ce8 
this makes it easier for the user to know what kind of URL to put in there, because you can view the example URL in the browser and see what it should contain

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

mnlipp commented Oct 1, 2024

@mnlipp are you ok with your repo appearing in the screenshot?

Only just seen that. No problem.

@chrisrueger
Copy link
Contributor Author

@pkriens how should we proceed here? I implemented all of what we discussed and decided in the call and adjusted the PR description accordingly.
Just merging it and give it a try?

@chrisrueger
remove wrapper class
db653e4 
not needed anymore after we discarded the "snapshot version" idea

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

pkriens commented Oct 2, 2024

Yup! Better to ask for a revert sometimes than to never fail :-)

Merge!

@chrisrueger chrisrueger merged commit d3bd71e into bndtools:master Oct 2, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Template Fragments UI: allow user to optionally choose the master/main branch of a template
3 participants
@chrisrueger @mnlipp @pkriens