By default, Bugzilla does not search the list of RESOLVED bugs.
You can force it to do so by putting the upper-case word ALL in front of your search query, e.g.: ALL tdelibs
We recommend searching for bugs this way, as you may discover that your bug has already been resolved and fixed in a later release.
Bug 1779 - Kate syntax highlight files update
Summary: Kate syntax highlight files update
Status: RESOLVED FIXED
Alias: None
Product: TDE
Classification: Unclassified
Component: tdebase (show other bugs)
Version: R14.0.0 [Trinity]
Hardware: Other Linux
: P5 normal
Assignee: Michele Calgaro
URL:
Depends on: 1804
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-23 07:29 CST by Michele Calgaro
Modified: 2014-01-08 20:13 CST (History)
4 users (show)

See Also:
Compiler Version:
TDE Version String:
Application Version:
Application Name:


Attachments
Patch to update kate syntax files (965.66 KB, patch)
2013-12-26 13:33 CST, Darrell
Details | Diff
updated patch (963.13 KB, patch)
2013-12-27 07:37 CST, Michele Calgaro
Details | Diff
final patch (1.00 MB, patch)
2013-12-28 09:26 CST, Michele Calgaro
Details | Diff
Updated tdelibs kate syntax patch (1.00 MB, patch)
2013-12-28 11:07 CST, Darrell
Details | Diff
Updated tdelibs kate syntax patch. (1.01 MB, patch)
2013-12-28 19:58 CST, Darrell
Details | Diff
updated patch (1.02 MB, patch)
2013-12-29 01:19 CST, Michele Calgaro
Details | Diff
patch to be used for local testing (NOT TO BE PUSHED TO GIT!!) (1.02 MB, patch)
2014-01-02 08:45 CST, Michele Calgaro
Details | Diff
updated patch for local testing (NOT TO BE PUSHED TO GIT!!) (1.02 MB, patch)
2014-01-03 07:17 CST, Michele Calgaro
Details | Diff
updated patch (to be pushed to GIT if no problem found) (1.02 MB, patch)
2014-01-03 07:20 CST, Michele Calgaro
Details | Diff
updated patch for local testing, post bug 1804 (NOT TO BE PUSHED TO GIT!!) (1.02 MB, patch)
2014-01-05 08:49 CST, Michele Calgaro
Details | Diff
updated patch, post bug 1804 (to be pushed to GIT if no problem found) (1.02 MB, patch)
2014-01-05 08:50 CST, Michele Calgaro
Details | Diff
updated GIT patch (replaces tree with plain) (1.02 MB, patch)
2014-01-06 23:50 CST, Michele Calgaro
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michele Calgaro 2013-12-23 07:29:24 CST
Kate syntax highlight files need to be updated.
Comment 1 Darrell 2013-12-26 13:32:01 CST
After comparing files, I think the following is what needs to be done to the syntax files after pulling from the kate-editor.org.syntax collection:

This file is part of KDE's kate project. -> This file is part of TDE's kate project.

kateversion="[1-2].[x.x]" -> kateversion="2.5"

khtml -> tdehtml

kfile->tdefile

part of kdelibs/kate -> part of tdelibs/kate

~/.kde -> ~/.trinity

TQt:
      <item> TQ_CLASSINFO </item>
      <item> TQ_ENUMS </item>
      <item> TQ_OVERRIDE </item>
      <item> TQ_PROPERTY </item>
      <item> TQ_SETS </item>
      <item> TQ_ARG </item>
      <item> TQ_ASSERT </item>
      <item> TQ_ASSERT_X </item>
      <item> TQ_CHECK_PTR </item>
      <item> TQ_CLASSINFO </item>
      <item> TQ_CLEANUP_RESOURCE </item>
      <item> TQ_D </item>
      <item> TQ_DECLARE_FLAGS </item>
      <item> TQ_DECLARE_INTERFACE </item>
      <item> TQ_DECLARE_METATYPE </item>
      <item> TQ_DECLARE_OPERATORS_FOR_FLAGS </item>
      <item> TQ_DECLARE_PRIVATE </item>
      <item> TQ_DECLARE_PUBLIC </item>
      <item> TQ_DECLARE_SHARED </item>
      <item> TQ_DECLARE_TYPEINFO </item>
      <item> TQ_DISABLE_COPY </item>
      <item> TQ_EMIT </item>
      <item> TQ_ENUMS </item>
      <item> TQ_EXPORT </item>
      <item> TQ_FLAGS </item>
      <item> TQ_FOREACH </item>
      <item> TQ_FOREVER </item>
      <item> TQ_GADGET </item>
      <item> TQ_GLOBAL_STATIC </item>
      <item> TQ_GLOBAL_STATIC_WITH_ARGS </item>
      <item> TQ_INIT_RESOURCE </item>
      <item> TQ_INTERFACES </item>
      <item> TQ_INVOKABLE </item>
      <item> TQ_NOREPLY </item>
      <item> TQ_OBJECT </item>
      <item> TQ_OVERRIDE </item>
      <item> TQ_PRIVATE_SLOT </item>
      <item> TQ_PROPERTY </item>
      <item> TQ_Q </item>
      <item> TQ_RETURN_ARG </item>
      <item> TQ_SCRIPTABLE </item>
      <item> TQ_SETS </item>
      <item> TQ_SIGNAL </item>
      <item> TQ_SIGNALS </item>
      <item> TQ_SLOT </item>
      <item> TQ_SLOTS </item>
      <item> TQ_UNUSED </item>
      <item> TQT_SIGNAL </item>
      <item> TQT_SLOT </item>
      <item> TQT_TQOBJECT </item>
Comment 2 Darrell 2013-12-26 13:33:37 CST
Created attachment 1747 [details]
Patch to update kate syntax files

There are additional TQt items that could be added. I haven't tried to build a robust list.

Attached is a patch to update the existing syntax files.
Comment 3 Michele Calgaro 2013-12-26 21:40:41 CST
> Attached is a patch to update the existing syntax files.
Thanks Darrell, well done. The last few days I have been extremely busy and had no time for development at all, just been able to keep up with the mailing list (and not that well overall) :(

Tonight I should have some time, so I will check this as well. Your patch is a great starting point, should spare me a lot of time. We need to double check it for silly mistakes. For example from a very (very!) quick look through it I spot a small mistake in the last line: "lockfile" should remain "lockfile" and not "loctdefile" I believe. Perhaps there could be a few more.
Comment 4 Darrell 2013-12-26 22:06:41 CST
>"lockfile" should remain "lockfile" and not "loctdefile" I believe.
Exactly why I attached the patch rather than push to git as something "intuitvely obvious." Like any project, we've seen our share of presumptive mistakes, made by all of us at one time or another. I've made more than my share. :)
Comment 5 Michele Calgaro 2013-12-27 07:37:38 CST
Created attachment 1750 [details]
updated patch

Updated patch.
Compared to Darrell's patch, I fixed a few lockfile -> loctdefile unwanted renames (bash.xml, tcsh.xml, zsh.xml), removed a few double entries in cpp.xml, added TQ* entries in objectivecpp.xml (not sure there are needed, but no harm) and added a few KDE -> TDE renaming.
Thanks to Darrell for his good job! This patch is 99.99% his merit.
Comment 6 Darrell 2013-12-27 12:53:10 CST
We probably should randomly open many Trinity cpp/h source files and look for items that are not highlighted, which could be added to the two cpp xml files.

We still need a way to automate Trinity customizations, even if a single person manually performs all of the tweaks.
Comment 7 Michele Calgaro 2013-12-28 09:26:54 CST
Created attachment 1768 [details]
final patch

This is the final patch.
Changes compared to previous patch:
1) changed download location from "http://kate.kde.org/syntax/update-2.5.xml" to "http://git.trinitydesktop.org/cgit/tdelibs/tree/kate/data/update-files.xml", so that TDE will have its own syntax files
2) added file update-files.xml to the tdelibs/tree/kate/data folder. This file is the equivalent of the update-2.5.xml used for the KDE syntax files and contains the definition of the TDE syntax files.
3) added TQ* symbols to C++11 and objectivecpp
4) fixed unwanted rename in stata.xml

To test this patch "as is", it is necessary for the patch to be pushed to GIT, since the update-files.xml file will be downloaded when trying to update the files from the Kate GUI.
Anyhow you can pre-test the patch with a simple change to the downlaod location. After applying the patch, open the file tdelibs/kate/part/katedialogs.cpp and at the beginning modify the line 

#define HLDOWNLOADPATH "http://git.trinitydesktop.org/cgit/tdelibs/tree/kate/data/update-files.xml"

to point to the folder in your system which stores the syntax files.
Comment 8 Michele Calgaro 2013-12-28 09:29:37 CST
> Anyhow you can pre-test the patch with a simple change to the downlaod
> location. After applying the patch, open the file
> tdelibs/kate/part/katedialogs.cpp and at the beginning modify the line 
> 
> #define HLDOWNLOADPATH
> "http://git.trinitydesktop.org/cgit/tdelibs/tree/kate/data/update-files.xml"
> 
> to point to the folder in your system which stores the syntax files.

I forgot to mention that you also need to modify the path of each file in update-files.xml to point to your local folder.


Also I forgot to mention the I updated the file version number of a few modified files (excluding simple renames).
Comment 9 Darrell 2013-12-28 09:47:44 CST
I notice some of the xml files contain references to kate.kde.org. Now that we are updating the files to use Trinity sources, should our Trinity tweaks include deleting those references from the affected xml files?

>to point to the folder in your system which stores the syntax files.
What is the correct syntax for a local URL? I presume replace http:// -> file:// and then use the full patch to the local git tree?
Comment 10 Michele Calgaro 2013-12-28 10:08:38 CST
(In reply to comment #9)
> I notice some of the xml files contain references to kate.kde.org. Now that we
> are updating the files to use Trinity sources, should our Trinity tweaks
> include deleting those references from the affected xml files?

I noticed that as well and had the same thought. But there would also be some other things to remove here and there. I think we can live with it for the time being, but if you want to remove those go ahead :)

> 
> >to point to the folder in your system which stores the syntax files.
> What is the correct syntax for a local URL? I presume replace http:// ->
> file:// and then use the full patch to the local git tree?

file:///path/to/folder/filename.ext

To test, open your browser and type that in ;)
Comment 11 Darrell 2013-12-28 10:38:14 CST
I never did understand the need for three slashes.

The testing requirements has me thinking, which often is dangerous:

Add a check box just above the Download button:

[ ] Download from a local source

The option always opens unchecked. When checked and the user selects the Download button, a dialog appears asking for the file path, which could be our local git tree or could be an optical disk.

The dialog then changes the HLDOWNLOADPATH variable to the user's selected local path.

WhatsThis help tooltip:

When selected Kate will download updated syntax highlighting files from a local source, such as an optical disk.
Comment 12 Darrell 2013-12-28 11:07:26 CST
Created attachment 1784 [details]
Updated tdelibs kate syntax patch

> I think we can live with it for the time being, but if you want to remove
> those go ahead :)
I did. I updated a couple of other references in tdelibs as well.

Updated patch attached.
Comment 13 Darrell 2013-12-28 19:58:11 CST
Created attachment 1785 [details]
Updated tdelibs kate syntax patch.

Patch updated again. We needed to add all of the new xml files to the CMakeLists.txt file. There were 30 new xml files. Some of the xml files are in the existing git sources but were not in the CMakeLists.txt and thus never got built into the new package. Nobody noticed.

I discovered this abruptly after installing an updated tdelibs package from today. Kate started throwing rude dialogs at me. I traced the problem to missing xml files, although I don't know exactly why the missing files caused kate to belch so rudely. Perhaps there is a master file that tracks what should be installed.

Different topic: Does kate have a mechanism to cross-check system xml files in $PREFIX/share/apps/katepart/syntax to those installed in $TDEHOME/share/apps/katepart/syntax? Users could download updates in between maintenance releases. Then with the next maintenance release have exact duplicates in $TDEHOME. Seems there should be some kind of housekeeping to remove the exact duplicate files from $TDEHOME?
Comment 14 Michele Calgaro 2013-12-29 01:19:07 CST
Created attachment 1786 [details]
updated patch
Comment 15 Michele Calgaro 2013-12-29 01:37:26 CST
> Patch updated again. We needed to add all of the new xml files to the
> CMakeLists.txt file. There were 30 new xml files. Some of the xml files are in
> the existing git sources but were not in the CMakeLists.txt and thus never got
> built into the new package. Nobody noticed.

Actually I noticed that when I was checking all the xml files, but by the time I got to the end of the list, I forgot (perhaps I stayed up too late yesterday night :) )

The new updated patch lists the files in alphabetical order in CMakeLists.txt and also updates Makefile.am, which was also missing several xml files.
Comment 16 Michele Calgaro 2013-12-29 08:11:14 CST
> Different topic: Does kate have a mechanism to cross-check system xml files in
> $PREFIX/share/apps/katepart/syntax to those installed in
> $TDEHOME/share/apps/katepart/syntax? Users could download updates in between
> maintenance releases. Then with the next maintenance release have exact
> duplicates in $TDEHOME. Seems there should be some kind of housekeeping to
> remove the exact duplicate files from $TDEHOME?

I did a few tests and Kate seems to always use a file from $TDEHOME/share/apps/katepart/syntax if a file is present (regardless of its version), otherwise it will use the one from $PREFIX/share/apps/katepart/syntax.
If a user already has some local copies, it is not a big problem. In fact that is the reason why I increased the version of the files we modified (except for canonical renaming). So if the user updates the files again, the new version will be downloaded and will overwrite the old local version.
NOTE: for cpp.xml I changed the number from 1.55 to 1.52. KDE version is 1.51 and so far users could only download from the kate syntax website. So there is no way a user can have a local copy with number 1.55, unless it copied it on purpose.

--------------

> Add a check box just above the Download button:
> [ ] Download from a local source
> The option always opens unchecked. When checked and the user selects the
> Download button, a dialog appears asking for the file path, which could be our
> local git tree or could be an optical disk.

It's a nice idea but IMO I think it is more than what we need. Files in the TDE GIT repository will be updated regularly (I am planning to check once a month), so once this patch is pushed, all R14 users will have regular updates.
If a user wants to use a local copy, it just needs to copy the xml files to $TDEHOME/share/apps/katepart/syntax and Kate will take care of everything.
Comment 17 Darrell 2013-12-29 11:51:22 CST
>I did a few tests and Kate seems to always use a file from
>$TDEHOME/share/apps/katepart/syntax if a file is present
That is my experience as well, after many years of using kate. That means when the user's version is older than the system version, the older version still gets used, even when the user manually performs a download update from within kate.

That is why I raised the topic of basic housekeeping, at least for those copies that are exact duplicates of the system version. When `diff user_version system_version = ""` then delete user_version. The download feature could also compare the user's version to the system version. When the system version is newer than the user's, delete the user's version.

What about corner cases where a knowledgeable user manually edits the user's copy of an xml file? A housekeeping scheme would delete those manual tweaks. Well, the user should submit a bug report to get the master system file updated. :)

>In fact that is the reason why I increased the version of the files we modified
As I browsed the upstream copies, I noticed the versions were bumped by only 0.01. Upstream maintainers don't use an easy system like bumping 0.05 or 0.1. When we bump the version in our sources, our (eventual) updating script will not see a difference from upstream. Our (eventual) updating script can't use version numbers to determine whether an upstream file has changed. Comparing version numbers will still work for users with the kate download feature, but not with our (eventual) updating script.

Considering how long we have been hacking at this one patch, we need a script to help us automate this task. As we both discovered, adding new files from upstream means we need to update our local CMakeLists.txt and Makefile.am configuration files as well as the other Trinity related tweaks.

Idea: Let's split the patch into several patches. The first patch contains only a few xml file updates, all changes to all *.cpp/*.h, and respective changes to configuration files of only those xml files we push the first time. That way we can test the download feature from the new git location. As the downloaded files are stored in the user's $TDEHOME, we will know exactly what should and does get downloaded. We then push incremental sets of patches with a few xml file changes in each patch and rspective CMakeLists.txt?Makefile.am changes. This incremental updating to the git sources allows us to test in a robust way.

If we push everything all at once we then have only one shot of testing.

Pushing a few xml updates in increments also provides us an opportunity to test our (eventual) updating script. We have 70+ xml changes, which provides us plenty of files to fine-tune everything.
Comment 18 Darrell 2013-12-29 15:42:40 CST
I split the single big patch into 14 patches:

tdelibs-kate-syntax-base.diff
tdelibs-kate-syntax-xml-ab.diff
tdelibs-kate-syntax-xml-cd.diff
tdelibs-kate-syntax-xml-ef.diff
tdelibs-kate-syntax-xml-gh.diff
tdelibs-kate-syntax-xml-ij.diff
tdelibs-kate-syntax-xml-kl.diff
tdelibs-kate-syntax-xml-mn.diff
tdelibs-kate-syntax-xml-op.diff
tdelibs-kate-syntax-xml-qr.diff
tdelibs-kate-syntax-xml-st.diff
tdelibs-kate-syntax-xml-uv.diff
tdelibs-kate-syntax-xml-wx.diff
tdelibs-kate-syntax-xml-yz.diff

We could update the following and then work on the update script:

tdelibs-kate-syntax-base.diff
tdelibs-kate-syntax-xml-ab.diff

Then push one patch at a time to refine the update script.

I haven't uploaded the new patch set.

==============================
Kate syntax maintenance script
==============================

* Download xml files from kate-editor.org/syntax/2.5

* Compare update-2.5.xml to update-files.xml and update update-files.xml

* Update: CMakeLists.txt and Makefile.am with new xml files and sort lists

* Update: This file is part of KDE's kate project. -> This file is part of TDE's kate project.

* Update: kateversion="[1-2].[x.x]" -> kateversion="2.5"

* Update: "version="

* Update: khtml -> tdehtml

* Update: kfile->tdefile

* Update: loctdefile -> lockfile

* Update: part of kdelibs/kate -> part of tdelibs/kate

* Update: ~/.kde -> ~/.trinity

* Update: Unwanted renames

* Update: In cpp.xml, objective-cpp.xml, C++11, and objectivecpp, add Trinity TQt items

* Update: Remove duplicate items

======================
Special testing patch:
======================

* Modify update-files.xml:
  http://git.trinitydesktop.org/cgit/tdelibs/tree/kate/data/
  to
  file:///path/to/folder/
* Modify tdelibs/kate/part/katedialogs.cpp:HLDOWNLOADPATH to point to file:///path/to/folder/update-files.xml
* Rebuild and install test version of tdelibs

I wonder: Rather than rebuild tdelibs, a tester could instead retain a special package of modified files. Backup the affected files and overwrite the files with the files updated to file:///path/to/folder/. Looks like only two files: /opt/trinity/lib/trinity/libkatepart.so, /opt/trinity/share/apps/katepart/syntax/update-files.xml.
Comment 19 Michele Calgaro 2013-12-29 19:56:47 CST
(In reply to comment #17)
> the older version still gets used, even when the user manually performs a
> download update from within kate.

That's not what I saw. If I do a download update, older versions are overwritten as long as I select the file for update

> What about corner cases where a knowledgeable user manually edits the user's
> copy of an xml file? A housekeeping scheme would delete those manual tweaks.
> Well, the user should submit a bug report to get the master system file
> updated. :)

IMO, housekeeping is tricky, for exactly the reason you mentioned above. In the same way as "should submit a bug report" a user "should periodically checking for updates" :)

> As I browsed the upstream copies, I noticed the versions were bumped by only
> 0.01. Upstream maintainers don't use an easy system like bumping 0.05 or 0.1.

The bunch up in version number is intended for a user to be able to detect a new version when it comes available, so he can download the updated file.
For the updating scripts, see the next comment
Comment 20 Darrell 2013-12-29 20:13:19 CST
>That's not what I saw. If I do a download update, older versions are
>overwritten as long as I select the file for update
Too tired I guess. I should have written:

That means when the user's version is older than the system version, the older version still gets used.
Comment 21 Michele Calgaro 2013-12-29 20:15:05 CST
Regarding the need for updating scripts, I am a little skeptical in this case.
Reasons for this are:

1) if we check the kde repository periodically (once a month in my intention),
there should be very few changes each time. Keeping a copy of the KDE files
from the last check (which I am doing) allows for quick compares and then only
the new changes have to be merged to the TDE repository

2) even though those changes are automated, we all agree that a final developer
check is required. Using automated scripts we will have to re-check all the
"already made" changes each time, which increase the required time for a
developer. All these changes had already been checked once, why should we keep
checking them again and again and again....?
It's the same as doing something like this: we do not change the KDE source
code, we create a ever growing list of patches and each time we apply all of
them to get the TDE code (think in terms of the all TDE code, not just the xml
files)

3) as the contents of the xml files change, the updated scripts may require
maintenance as well, which means additional code change and testing for the
scripts themselves.
Example: at least one file has repeated entries (not in order). The updating
scripts need ad-hoc code to remove the double entry. Then one day the KDE guys
find out and remove the double entry. Consequently we need to modify the
updating script and test it again.
Another example: the order of contents of cpp.xml is rearranged. The updating
scripts need to be modify to change the location where the TQ* items are
inserted


Considering all, IMO developing and maintaining updating scripts in this case
would require more overhead work than the benefits they would provide.
I don't expect major changes to the xml files every month. Rather I expect some
minor corrections/improvement here and there.
We have done the major part of the upgrade with the uploaded patches. This
should be the only time when such major checking was needed.
Just my 2 cents :)
Comment 22 Michele Calgaro 2013-12-29 20:17:30 CST
> That means when the user's version is older than the system version, the older
> version still gets used.

Correct. Should we bunch up the version of all files by 0.01 with this patch so that with R14 users will have all new versions available? I have been battled between doing this and not doing this.
Comment 23 Darrell 2013-12-29 20:37:10 CST
Life has a funny way of making us busy or distracting us. :) When the files stop being maintained then all of the problems we discussed are reintroduced because the next person doesn't know better. A person not in the know will download the files from kate-editor.org/syntax and do a mass merge. Bang. We both know this WILL happen. :)

I'm fine with no automation script. Perhaps then we need a mini how-to that instructs others how to update the files. Wherever we store the mini how-to, possibly in kate/data/README, every xml file probably should have a comment near the very top pointing to that mini how-to acting as reminder how to update.

We should bump the xml version by 0.01 only for those files that have changed since 3.5.13.2. Don't attach any new patches yet. I'm rebuilding tdelibs with a local directory to start testing.
Comment 24 Michele Calgaro 2013-12-29 21:04:32 CST
> Life has a funny way of making us busy or distracting us. :)

I had the same thought this morning ;)

> Perhaps then we need a mini how-to that
> instructs others how to update the files. Wherever we store the mini how-to,
> possibly in kate/data/README, every xml file probably should have a comment
> near the very top pointing to that mini how-to acting as reminder how to
> update.

The mini how-to in kate/data/README is a good idea.

> We should bump the xml version by 0.01 only for those files that have changed
> since 3.5.13.2. Don't attach any new patches yet. I'm rebuilding tdelibs with a
> local directory to start testing.

Ok. Since we changed the kateversion to 2.5 in almost all files, I think 90% of them will require a small bunch up
Comment 25 Darrell 2013-12-29 21:45:51 CST
My testing using a local directory is not working. And I'm too tired to think straight anymore....
Comment 26 Michele Calgaro 2013-12-29 22:07:31 CST
(In reply to comment #25)
> My testing using a local directory is not working. And I'm too tired to think
> straight anymore....

No worries. Tonight or tomorrow at most I will update my building scripts and then I will test the patch locally. To be fair, this is the only patch I hadn't tested before submitting. Thanks for all your support so far :)
Comment 27 Darrell 2013-12-30 10:09:10 CST
I think to test this locally requires building tdelibs twice. First with the URL pointing to a local directory. This is to generate a libkatepart.so and update-files.xml containing the local directory URL. After compiling, copy those two files to a safe location.

Then compile tdelibs again with no syntax patching at all. After installing the unpatched tdelibs, copy/overwrite the testing version of libkatepart.so and the new update-files.xml to /opt/trinity/share/apps/katepart/syntax/.

Then copy all xml files to the local testing URL.

But this doesn't work either. The kate dialog shows all files that need updating but nothing happens when I select the Install button. Nothing gets copied to $TDEHOME and no "copying" dialogs appear.
Comment 28 Darrell 2013-12-30 17:41:51 CST
I don't know whether this is related, but seems of late that kate forgets all of my syntax highlighting settings of recently opened files. I have kate configured to retain metadata for 180 days.
Comment 29 Darrell 2013-12-30 17:44:42 CST
Oops, hit the Save button to soon. To be more specific, even if the 180 period has expired, kate seems incapable of automatically recognizing certain file types. Today I opened several xml files and I had to manually set the syntax highlighting of each one. Perhaps only xml files are affected. Perhaps other formats affected too.
Comment 30 Darrell 2013-12-31 15:47:35 CST
>I don't know whether this is related, but seems of late that kate forgets all
>of my syntax highlighting settings of recently opened files.
I found the problem. As is typical in the computer world, developers have to strictly enforce NIH (Not Invented Here). The VirtualBox developers had for years assigned xml as the file extension to all VirtualBox configuration files, which are indeed xml files.

I don't know which VirtualBox version, I think 4.x, they changed that file extension from xml to vbox. After all, NIH!

The declaration at top of the vbox files is as clear as can be that the file is an xml file. Unfortunately neither the kate xml.xml syntax file recognizes the vbox extension nor does the Trinity file association scheme recognize vbox as an xml mimetype.

We could modify the kate xml.xml file but as the xml.xml syntax file already recognizes the xml mimetype, seems more appropriate in the Trinity file association scheme to add vbox to the list of known xml mimetype file extensions.

I can do that for myself personally, but as popular as VirtualBox has become, I recommend we configure the Trinity file association defaults to recognize vbox file extension as an xml mimetype.

Adding vbox as an xml mimetytpe requires modifying tdelibs/mimetypes/text/xml.desktop in the source tree, which is installed as /opt/trinity/share/mimelnk/text/xml.desktop.
Comment 31 Michele Calgaro 2014-01-01 08:36:05 CST
> We could modify the kate xml.xml file but as the xml.xml syntax file already
> recognizes the xml mimetype, seems more appropriate in the Trinity file
> association scheme to add vbox to the list of known xml mimetype file
> extensions.
> I can do that for myself personally, but as popular as VirtualBox has become, I
> recommend we configure the Trinity file association defaults to recognize vbox
> file extension as an xml mimetype.

I favor the idea of adding vbox to the list of known xml files :) I see you open bug 1804 for that.

BTW, I am in the middle of a full TDE rebuild. Tomorrow I should be able to test the patch for Kate xml files locally.
Comment 32 Michele Calgaro 2014-01-02 08:45:55 CST
Created attachment 1822 [details]
patch to be used for local testing (NOT TO BE PUSHED TO GIT!!)

I finally completed a full TDE rebuild and today I tested this patch extensively.

> But this doesn't work either. The kate dialog shows all files that need
> updating but nothing happens when I select the Install button. Nothing gets
> copied to $TDEHOME and no "copying" dialogs appear.

Indeed it didn't work when tested locally. This is because the code in TDE uses "TDEIO::NetAccess::download()" which does nothing in case the URL is a local file. I have prepared a modified patch to be used for local testing. This makes sure files are copied in ~/.trinity/share/apps/katepart/syntax.

The original patch is ok though, since files would be downloaded from the TDE git server and so the TDE code would work fine as it did so far.

While testing I found out that the file C++11.xml is never shown as installed and is not available in the list of possible highlight syntaxes.
I am working on this, but can't finish today. Tomorrow I expect to complete it and upload a new patch to be pushed to GIT.
Comment 33 Michele Calgaro 2014-01-02 08:53:42 CST
Added dependency from bug 1804. If one of the two patches is push to GIT, the other patch needs to be updated
Comment 34 Darrell 2014-01-02 11:38:36 CST
>Indeed it didn't work when tested locally. This is because the code in TDE uses
>"TDEIO::NetAccess::download()" which does nothing in case the URL is a local
>file.
We could still add that check box to use a local source. :)
Comment 35 Michele Calgaro 2014-01-02 21:53:53 CST
> We could still add that check box to use a local source. :)
I can be done but I think it's more than what is needed. Most users just want to click "Download" and update the files, they won't bother about downloading files themselves and use a local folder. And if they want to use some specialized xml file, they just need to copy it to ~/.trinity/share/apps/katepart/syntax
We could make it an enhancement request though :) 

Going back to housekeeping, I think we should avoid it since a user may want to stick to an older version of an xml file for some reasons. But in the press release of R14 we need to add that "the syntax highlight file repository was moved to our GIT repository and the files have been updated to the latest version. Users are invited to remove local copies from ~/.trinity/share/apps/katepart/syntax. Files will be regularly updated (approximately once a month) if new versions become available"
Comment 36 Darrell 2014-01-02 21:59:50 CST
>they won't bother about downloading files themselves and use a local folder.
My focus was not on us (although that would help with testing), as much as users being able to update files from a removable or optical disk.
Comment 37 Michele Calgaro 2014-01-03 07:17:37 CST
Created attachment 1825 [details]
updated patch for local testing (NOT TO BE PUSHED TO GIT!!)

Updated patch usable for local testing.
I bunched up all modified files (even simple rename or Kate version update) by a 0.01 and fixed the problem with the C++11 xml file.
Comment 38 Michele Calgaro 2014-01-03 07:20:16 CST
Created attachment 1826 [details]
updated patch (to be pushed to GIT if no problem found)

Equivalent patch which should later be pushed to GIT
Comment 39 Michele Calgaro 2014-01-03 07:28:18 CST
(In reply to comment #36)
> >they won't bother about downloading files themselves and use a local folder.
> My focus was not on us (although that would help with testing), as much as
> users being able to update files from a removable or optical disk.

Added wishlist bug 1808
Comment 40 Michele Calgaro 2014-01-05 08:49:42 CST
Created attachment 1836 [details]
updated patch for local testing, post bug 1804 (NOT TO BE PUSHED TO GIT!!)

This patch can be applied without modifications after the patches for bug 1804 have been applied
Comment 41 Michele Calgaro 2014-01-05 08:50:41 CST
Created attachment 1837 [details]
updated patch, post bug 1804 (to be pushed to GIT if no problem found)

This patch can be applied without modifications after the patches for bug 1804 have been applied
Comment 42 Darrell 2014-01-05 19:41:53 CST
How are you testing this? All xml files are updated in the patch and get installed. Selecting the kate Download button will result in nothing happening, even from a local source.
Comment 43 Michele Calgaro 2014-01-05 21:29:55 CST
(In reply to comment #42)
> How are you testing this? All xml files are updated in the patch and get
> installed. Selecting the kate Download button will result in nothing happening,
> even from a local source.

There are two ways.
1) double compilation as you explained in comment 27

2) compile once using the "local" patch and install. All files are instantly updated. 
2.1) Simulate a new upstream version
Take one of the xml files, copy it to ~/.trinity/share/apps/katepart/syntax and edit its version to a lower number. Relaunch Kate (it's important to exit and relaunch!), go to settings highlight section and click download. The window with the list of files opens and for the modified file you should see a difference in the version numbers. Select the file, click "install" and check that the updated version has been copy to  ~/.trinity/share/apps/katepart/syntax, overwriting your previous version.

2.2) Silumate a local newer version
Take one of the xml files, copy it to ~/.trinity/share/apps/katepart/syntax and edit its version to a higher number. Relaunch Kate (it's important to exit and relaunch!), go to settings highlight section and click download. The window with the list of files opens and for the modified file you should see a difference in the version numbers.
Comment 44 Darrell 2014-01-06 11:43:41 CST
I ran some quick tests as suggested and all seems well.

An observation before pushing to git. When the user's version is newer than the system version, there is no highlighting in the Download dialog to help the user notice the difference. The Download dialog highlights differences only when the system file is newer than the user's. Can we fix that so any difference is highlighted?

Files that are highlighted in the dialog get updated when the user selects the Install button. That would include overwriting a user's newer xml file with an older system file. Perhaps a confirmation dialog is appropriate for those few/occasional exceptions?
Comment 45 Slávek Banko 2014-01-06 15:08:11 CST
I assume that the in update-files.xml should be paths
with "plain" instead of "tree". For example:

http://git.trinitydesktop.org/cgit/tdelibs/plain/kate/data/abc.xml


In the same way, the path to the update file should be:

http://git.trinitydesktop.org/cgit/tdelibs/plain/kate/data/update-files.xml
Comment 46 Darrell 2014-01-06 15:22:56 CST
Yes, 'plain' versus 'tree' makes sense to me.

I have not tested the git version of the patch because we need to upload the modified xml files for that to function correctly.

I notice in the patch the following:

+#define HLDOWNLOADPATH "http://git.trinitydesktop.org/cgit/tdelibs/tree/kate/data/update-files.xml"

I prefer the original code layout:

+#define HLDOWNLOADPATH "http://git.trinitydesktop.org/cgit/tdelibs/tree/kate/data/"

That is, HLDOWNLOADPATH is defined as just the path with no file name. Then retain part of the original code:

transferJob = TDEIO::get(
  KURL(TQString(HLDOWNLOADPATH)
     + TQString("update-files.xml")), true, true );
Comment 47 Michele Calgaro 2014-01-06 23:36:58 CST
> I assume that the in update-files.xml should be paths
> with "plain" instead of "tree". For example:
plain instead of tree is fine for me too.

> I prefer the original code layout:
> +#define HLDOWNLOADPATH
> "http://git.trinitydesktop.org/cgit/tdelibs/tree/kate/data/"
> That is, HLDOWNLOADPATH is defined as just the path with no file name. Then
> retain part of the original code:

Originally HLDOWNLOADPATH only contained the path name (without file name) because the full path/file name would be different for each new version of Kate.
Since we are switching the update path to our GIT repository, the path/file name no longer contains the version number. So I simplified the code by using the full path/file name directly in HLDOWNLOADPATH. Make the code easier to read and a few microseconds faster since there are no more string concat operations.
I am fine if you want to stick to the older version, but I don't see any advantage in that :)
Comment 48 Michele Calgaro 2014-01-06 23:47:05 CST
> An observation before pushing to git. When the user's version is newer than the
> system version, there is no highlighting in the Download dialog to help the
> user notice the difference. The Download dialog highlights differences only
> when the system file is newer than the user's. Can we fix that so any
> difference is highlighted?

As stated in the window that opens when clicking the Download button, new versions are selected automatically. This to simplify user life: a user doesn't need to check through the list to see what newer version are available. Just click Download and then Install.
If a user has a local version with higher number (because he manually modified it for example) he probably doesn't want to overwrite it with a newer upstream version. If we highlight it automatically, users may mistakenly do that.

IMO, the existing behavior is fine. I don't see the need of automatically selecting newer *local* version and then have to protect from unwanted overwriting by an additional dialog box. Plus, the user would have to manually deselect the local newer files each time prior to click Install, so I can already see someone filing a new bug report saying that newer *local* versions should not be highlighted by default :)

Anyhow if there are other people that share your opinion, we can open an enhancement/wishlist bug report for that.
Comment 49 Michele Calgaro 2014-01-06 23:50:18 CST
Created attachment 1842 [details]
updated GIT patch (replaces tree with plain)
Comment 50 Darrell 2014-01-07 20:41:36 CST
>I am fine if you want to stick to the older version, but I don't see any
>advantage in that :)
Not worth arguing about.

>updated GIT patch (replaces tree with plain)
Is this ready to push to git?
Comment 51 Michele Calgaro 2014-01-07 23:23:03 CST
> >updated GIT patch (replaces tree with plain)
> Is this ready to push to git?
Yes, unless Slavek finds something else he think it is worth being changed/fixed
Comment 52 Darrell 2014-01-08 00:52:51 CST
>Yes, unless Slavek finds something else he think it is worth being changed/fixed
Okay. I'm statisfied too.
Comment 53 Slávek Banko 2014-01-08 17:26:21 CST
Comment on attachment 1842 [details]
updated GIT patch (replaces tree with plain)

Pushed to GIT in hash bf4e405e.
Comment 54 Slávek Banko 2014-01-08 17:35:47 CST
We leave this bug report open, or (semi-) automation will be addressed in separate bug report?
Comment 55 Darrell 2014-01-08 17:48:24 CST
We can close this report. Automation is not going to happen soon.

Now that the patch is pushed, we have no direct method to check whether the KDE folks have updated their files. That process now has to be performed manually and the files cross-checked against our copies. Michele has agreed to manually oversee the updating process for the next few months.
Comment 56 Michele Calgaro 2014-01-08 20:13:55 CST
> Michele has agreed to manually oversee the updating process for the next few > months.

Yes, the plan is to check once a month and merge the new KDE changes if any.
If we see the volume of changes is consistently huge, we should discuss about automation again, but I don't expect that :)