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

Multi project extension #221

Closed
wants to merge 11 commits into from

Conversation

MysticJay
Copy link
Contributor

@MysticJay MysticJay commented Jul 10, 2019

This is the first and IMHO most important plugin ZasoGD has created.
The script itself is unchanged compared to the original file.
Headers and footers are changed (not sure about the @match line)

Zaso's startup wrapper has been commented out, although I do not yet understand why.

The script has been tested in IITC-CE - Menu item load.

no changes to Zaso's script.
Header, Tail
wrapper () removed
@MysticJay MysticJay changed the title Muti project extension Multi project extension Jul 10, 2019
Code Snippet for integrating other plugins with MPE
removed commented functions and @match
@MysticJay
Copy link
Contributor Author

IMHO ready to merge

@modos189
Copy link
Contributor

Contents of Multi-Projects-Extension.doc file please place as comment in multi-projects-extension.user.js file, and delete Multi-Projects-Extension.doc

@modos189 modos189 self-requested a review July 10, 2019 21:07
@modos189 modos189 added enhancement New feature or request plugin labels Jul 10, 2019
@MysticJay
Copy link
Contributor Author

MysticJay commented Jul 10, 2019

Does IITC-CE work with Block-Comments? /* and */?

Sample Code moved to main script
DOC removed
@modos189
Copy link
Contributor

You can use multiline comments. Yeah, like you just did.

We have documentation (in the doc folder) but I don't like it very much, it should be generated based on comments in the code.

@modos189
Copy link
Contributor

Header fixed,
Indentation reduced
modos189
modos189 previously approved these changes Jul 10, 2019
@modos189 modos189 added this to the v0.30 milestone Jul 10, 2019
@modos189
Copy link
Contributor

Thanks, this PR will be merged into a new release

@modos189 modos189 mentioned this pull request Jul 10, 2019
3 tasks
@johnd0e
Copy link
Contributor

johnd0e commented Jul 11, 2019

Thanks, this PR will be merged into a new release

I'd rather think about include support of this in IITC core.
May be in some other form (need to investigate functionality deeper).

Added MPE-Support for Drawtools.
@MysticJay
Copy link
Contributor Author

To test the functionality:

  1. import Multi-Project-Extension.user.js to externals
  2. import DRAW2-beta.user.js to externals
  3. disable regular DRAW-Tools
  4. enable MPE and Draw2-beta
    Create a drawing
    from MultiProjects Menu click on the "+" and create a new project.
    Existing drawing should be cleared.
    create a new draw.
    Use the dialogue to switch between drawings
    Use the settings dialogue to add the selection menu to the MainMenu.

Added MPE-Support to current Bookmarks.
import to extrenals and disable standard BMRKS-plugin to test
@MysticJay
Copy link
Contributor Author

To test:
disable standard Bookmarks-Plugin,
import bkmks2.beta to externals

@johnd0e
Copy link
Contributor

johnd0e commented Jul 30, 2019

  1. I do not get why you need separate plugin (draw2). You may just extend single draw-tools (in runtime)

  2. But if you still need draw2 then please make your changes reviewable:

    • copy draw-tools.user.js to draw-tools2.user.js
    • make your changes in separate commit(s)

@MysticJay
Copy link
Contributor Author

Bare with me, I am new to GIT, so it seemed to be safer to create a separate draw than to fiddle around in the original draw. At least this way does not break the existing tools and existing extensions to it. When I started porting Zaso's changes it was not clear to me how deep the changes would be.

The changes can easily be followed by just renaming DRAW2-beta.user.js to draw-tools.user.js
Also the contributions should show the changes step by step, as the first contribution of the file was the original draw-tools (just a copy)

Draw2 and Bkmarks2 is "only" a Draft to show how MPE is working. Due to the same namespace both use with the old Versions, the data is completely compatible. If you find a problem with MPE it is easy to switch back to the old plugins without loosing anything.

Also there are many other proposals that need to be reviewed and I think it is easier to add these to a forked plugin and finally just replace the old plugin than to daily send in new contributions.

As you wrote earlier we should rather think of adding MPE to the code as "Projects" or "Project-Manager".

@johnd0e
Copy link
Contributor

johnd0e commented Jul 30, 2019

Also the contributions should show the changes step by step, as the first contribution of the file was the original draw-tools (just a copy)

Yeah, but for draw-tools I see only single commit 9d89089

Better if there were (1) initial (2) changes for MPE

Also there are many other proposals that need to be reviewed and I think it is easier to add these to a forked plugin

We can play with forked plugins, but I do not think that we should release them as they are. It'd be confusing.

As you wrote earlier we should rather think of adding MPE to the code as "Projects" or "Project-Manager".

Yes. It is general principle.
But there may be different implementations of Multi-Projects idea.
I need time to check it.

Multi-Project support for Bookmarks and Draw-Tools added.
(imported changes from beta)
@MysticJay
Copy link
Contributor Author

Changes applied on original files. removed development (beta) files.

@johnd0e
Copy link
Contributor

johnd0e commented Jul 30, 2019

Ok. I will review proposed changes ASAP (but do not expect it to be fast, definitely not earlier than 0.30 release).

@johnd0e
Copy link
Contributor

johnd0e commented Jul 31, 2019

Some general thoughts:

MPE allows to group Draws/Bookmarks by related project.
What if we need some particular set of Draws or Bookmarks in several projects?
Oops. We can't.

I suppose we could improve project idea.
E.g.:
Bookmarks already has folders. So we might just select existent folders to add them into project.

@johnd0e johnd0e mentioned this pull request Aug 7, 2019
@@ -0,0 +1,30 @@
// ==UserScript==
// @id iitc-plugin-font-awesome@Zaso
// @name IITC plugin: Font Awesome
Copy link
Contributor

@johnd0e johnd0e Aug 7, 2019

Choose a reason for hiding this comment

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

If we include web font - we should include it in core, to make it available out-of-the-box.
(and it is still unclear if should include - #236)

$('<link>')
.prop('type', 'text/css')
.prop('rel', 'stylesheet')
.prop('href', 'https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css')
Copy link
Contributor

@johnd0e johnd0e Aug 7, 2019

Choose a reason for hiding this comment

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

Why use 4.7 instead of current (5.10.1)?
https://fontawesome.com/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No objection to use 5.10.1.
Just not tested it.

@MysticJay
Copy link
Contributor Author

MPE is about replacing the complete namespace of a plugin. Sharing parts between different namespaces would require them to be in an additional namespace.
Whatever I do not think this is needed Copy paste between different projects is possible and helps to avoid accidental sharing of confidential Information between projects.
I highly recommend to not share anything between projects.

@johnd0e
Copy link
Contributor

johnd0e commented Aug 8, 2019

I still don't get what do you mean. Explain please.

@MysticJay
Copy link
Contributor Author

You asked about sharing parts of draw or bookmarks between projects.

@MysticJay
Copy link
Contributor Author

What about moving this to the core?

@johnd0e
Copy link
Contributor

johnd0e commented Aug 31, 2019

It is planned

@johnd0e johnd0e dismissed modos189’s stale review October 10, 2019 06:23

There are some unresolved issues

@johnd0e johnd0e removed this from the v0.30 milestone Oct 10, 2019
Version alinged with ZasoItems-CE
@MysticJay MysticJay closed this Jan 15, 2020
@MysticJay MysticJay deleted the Muti-Project-Extension branch January 15, 2020 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants