gnosygnu / xowa

xowa offline wiki application
Other
375 stars 41 forks source link

TemplateStyles: Incorporate Xoh_css_minify from @desb42 #704

Closed gnosygnu closed 4 years ago

gnosygnu commented 4 years ago

As posted in #426

@desb42 added a cool piece of code to minify CSS for templateStyles in myxowa: https://github.com/desb42/myxowa/blob/master/400_xowa/src/gplx/xowa/htmls/Xoh_css_minify_v3.java

Will incorporate this code on my side. Main concerns are:

desb42 commented 4 years ago

Yes, unit tests are very important - I have not figured out how to use the unit tests (possibly due to the fact that I use netbeans?) I have tried using one of the Eclipse 2019 version, but could not work out how to set up a development environment for xowa [I have never used Java in a commercial environment]

Would it be possible to sneek a peek at your transpiler?

gnosygnu commented 4 years ago

Yes, unit tests are very important - I have not figured out how to use the unit tests (possibly due to the fact that I use netbeans?) I have tried using one of the Eclipse 2019 version, but could not work out how to set up a development environment for xowa

Yeah, unfortunately I'm pretty inexperienced with NetBeans, so don't know what to say. 145_ios.zip

I put together Eclipse specific instructions several years ago: http://xowa.org/home/wiki/Dev/Code#IDE_instructions_(Eclipse). They're a bit out of date (I use Eclipse Neon now), but the rest should still be enough to get you started.

Would it be possible to sneek a peek at your transpiler?

Sure, I attached it below. A lot of disclaimers first:

Would be happy to walk you through any particular points / questions, so feel free to ask! Thanks!

145_ios.zip

gnosygnu commented 4 years ago

Just two notes:

I'll update the http://xowa.org/home/wiki/Dev/Code documentation with an IntelliJ section sometime this week.

However, this will ripple changes throughout the XOWA code-base.

Question for @desb42 : What's the best way to coordinate massive find-replaces / reorganization of classes? I know I'm pretty far behind in incorporating your changes, so I don't want to disrupt https://github.com/desb42/myxowa. For now, I'm planning on keeping changes at an incremental level, and giving you a head's up when doing something dramatic.

desb42 commented 4 years ago

When I first came to this project, I tried to provide changes as branches of my copy of xowa. Unfortunately, I suspect I overwhelmed your ability to work on this project and incorporate my changes given that bills still need to be paid.

I am sure, you have lots of ideas for this project (as have I)

I regard myxowa as more of a testing ground for some ideas I have. Which then need to be carefully considered as to whether to incorporate them in xowa

I have introduced new classes that I suspect dont fit with your naming scheme (or coding style). You have the potentially harder task of maintaining backward compatibility (and unit tests)

As my code base is diverging and massive changes may be coming (whatever the interpretation of massive), I think the cleanest way would be (at some point in the future) to create a new project (xowaV5?)

Then decide between us the order of implementing the features from myxowa

I could then workup a branch that has the feature we agreed on, then you would have to integrate it (This still put a heavy burden on you)

gnosygnu commented 4 years ago

Unfortunately, I suspect I overwhelmed your ability to work on this project and incorporate my changes given that bills still need to be paid.

Yeah, I really fell short on my end. Unfortunately, work time has been worse the past few years than previously and personal time has also gone short. I'd like to match even a fraction of what you've done, but it's hard. Sorry about that.

I am sure, you have lots of ideas for this project (as have I)

Cool. Looking forward to hearing them!

I regard myxowa as more of a testing ground for some ideas I have. Which then need to be carefully considered as to whether to incorporate them in xowa

I think your code is really good caliber code for integration. The main problem on my side is structural. I wrote XOWA as its own hand-made parser. It's done a decent job of handling most of the MediaWiki wikitext, but I feel it's a fine balancing act. At this point, it needs to move to a more direct translation of the MW code. So, I'd rather spend more time patching direct (or easy) bugs than commit to riskier changes that can cause other bugs.

I have introduced new classes that I suspect dont fit with your naming scheme (or coding style).

Nah, I need to drop my naming scheme / coding style. ;)

You have the potentially harder task of maintaining backward compatibility (and unit tests)

Yeah, it's backward compatibility that's the rub. Unit tests are also good too.

As my code base is diverging and massive changes may be coming (whatever the interpretation of massive), I think the cleanest way would be (at some point in the future) to create a new project (xowaV5?)

Regarding the changes, I'll start by moving a lot of test classes out of the main src namespace and into a tst namespace. Depending on how that goes, I'll see what's next. Will let you know in advance before the massive test changes.

I'm hoping I can integrate these slowly into the main XOWA project. I'd rather not create a separate project, but if that works better for you, let me know.

Then decide between us the order of implementing the features from myxowa I could then workup a branch that has the feature we agreed on, then you would have to integrate it (This still put a heavy burden on you)

Yup, let's give it a trial run with something simple. If you want to identify others, I'll work on them after the cssmin one

Thanks!

gnosygnu commented 4 years ago

Took longer than I thought, but some growing pains with IntelliJ

Made a few changes from Xoh_css_minify_v3 which are enumerated below. Let me know if any objections. Otherwise, thanks!


desb42 commented 4 years ago

Commented out following regexs since it's handled in TemplateStyles and will be more performant there.

The purpose of the two lines is to ensure that all the selectors get .mw-parser-output added to them, not just the first one

Also, (a very minor point) Template_styles_nde.java is missing its copyright notice

desb42 commented 4 years ago

And it get worse! Looking at en.wikipedia.org/wiki/Buenos_Aires and comparing the templatestyles for this page in xowa and mediawiki

This shows that .mw-parser-output is added to every selector However, there is also an additional class (.tmulti) added to the template Template:Multiple image/styles.css This is indicated in Module:Multiple image when constructing a Templatestyle on line 293. This adds an extra attribute wrapper that I have not seen before This has the effect of adding .mw-parser-output and the wrapper class

gnosygnu commented 4 years ago

This shows that .mw-parser-output is added to every selector

Ah, ok. Will uncomment those two lines.

However, there is also an additional class (.tmulti) added to the template Template:Multiple image/styles.css This is indicated in Module:Multiple image when constructing a Templatestyle on line 293. This adds an extra attribute wrapper that I have not seen before

Ugh. Let me investigate this.

Also, (a very minor point) Template_styles_nde.java is missing its copyright notice

Yeah, I was hoping that the LICENSE.txt would be enough. In addition, I needed to update the year range on the copyright notice (Copyright (C) 2012-2017) and didn't want to spam updates to every file.

Will double-check and see if that's ok. Otherwise, will add back.

desb42 commented 4 years ago

I have just installed Intellij Community Edition but still have been unable to work out how to get this environment to build xowa

Any chance of sharing the project files?

gnosygnu commented 4 years ago

Any chance of sharing the project files?

Hey. I just zipped my git root and uploaded it to my google drive here: https://drive.google.com/file/d/1CXNqzPQBJOfV-17_IuyyU6xnlpc9egbJ/view?usp=sharing

I took a quick look at the .iml files, and there is some links to libraries outside the root. You'll probably have to fix those.

Otherwise, will try to fix, test on my laptop, and upload them as part of VCS later.

Let me know if you have other issues! Thanks!

gnosygnu commented 4 years ago

Yeah, I was hoping that the LICENSE.txt would be enough. In addition, I needed to update the year range on the copyright notice (Copyright (C) 2012-2017) and didn't want to spam updates to every file.

Will double-check and see if that's ok. Otherwise, will add back.

Also, FYI, I created this issue: https://github.com/gnosygnu/xowa/issues/711

In short, will add back the per-file licenses. Thanks for pointing that out!

gnosygnu commented 4 years ago

Hey, I made another commit for the following:

However, there is also an additional class (.tmulti) added to the template Template:Multiple image/styles.css This is indicated in Module:Multiple image when constructing a Templatestyle on line 293. This adds an extra attribute wrapper that I have not seen before

Ugh. Let me investigate this.

Yup, it looks like TemplateStyles supports a wrapper argument. See https://github.com/wikimedia/mediawiki-extensions-TemplateStyles/blob/master/includes/TemplateStylesHooks.php#L277

I created a new issue to track this: #712

Planning to close this issue out in the meantime. Let me know if you have any objections.

Also, hoping IntelliJ is going okay. Let me know if I can help in any way.

Thanks!

desb42 commented 4 years ago

Thanks for the dump; however, I am unable to get anything to work

I take the zip file and expand the xowa directory to my D: drive (so I end up with D:\xowa) I start up Intellij, click on Open or Import and select D:\xowa, there is a little bit of babbling then I expand the xowa tab - I get intellij1

If I try the build tab, nothing happens (I think there should be more colour in the project)

I know this is a little off the xowa track - however, any hints?

gnosygnu commented 4 years ago

Sorry about that. The zip file wasn't "IntelliJ" ready

Took some time just now (lunch break) to quickly put together another zip. This works when I unzip it to another drive (haven't tried on another machine). Hopefully it just works for you.

https://drive.google.com/file/d/1PBC_hmWVrcleS2T01dQZAyaUrxh_Ijfl/view?usp=sharing

Let me know if still issues. Thanks!


Zip instructions

Manual IntelliJ instructions (if you want to start from the xowa.zip file):

desb42 commented 4 years ago

success! The last zipfile did the trick

gnosygnu commented 4 years ago

Cool! Good to hear!

I created #713 above to track IntelliJ set-up . Feel free to ping me there for more questions. Will try to do general documentation within the next week or so.

Thanks!