GNOME Bugzilla – Bug 789021
Syntax Highlighting: Add android logcat language and test files
Last modified: 2017-10-21 08:42:06 UTC
Created attachment 361622 [details] [review] Patch file to add android logcat syntax highlighting and test This adds syntax highlighting for android logcat files. test file included for review
Created attachment 361632 [details] [review] Just a slight update to the previous patch which allows for 5 digit thread IDs
Review of attachment 361632 [details] [review]: Thanks for your patch. The *.lang file should be added to: data/language-specs/Makefile.am po/POTFILES.skip More comments below. ::: data/language-specs/logcat.lang @@ +21,3 @@ + You should have received a copy of the GNU Lesser General Public + License along with this library; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA Copy the same license header as for example c.lang from git master. The physical FSF address should be replaced by an URL. @@ +24,3 @@ + +--> +<language id="logcat" _name="logcat" version="2.0" _section="Others"> _name -> name, to not translate it (it has been changed recently on git master for all *.lang files). Others -> Other, the sections have been renamed several years ago. @@ +38,3 @@ + <style id="error" _name="Error" map-to="def:number"/> + <style id="fatal" _name="Fatal" map-to="def:error"/> + <style id="others" _name="Others" map-to="def:comment"/> For all <style> elements: _name -> name, to not translate them. @@ +47,3 @@ + <start>^---------</start> + <end>$</end> + </context> The main context (here with id="logcat") should include other contexts by reference, not by defining them inside. See for example the bottom of c.lang.
Created attachment 361701 [details] [review] amended based on comments Thank you for the review. I think I got everything but please let me know if you have other recommendations.
Review of attachment 361701 [details] [review]: You forgot to add the file to Makefile.am and POTFILES.skip as I said previously… I've pushed this branch: https://git.gnome.org/browse/gtksourceview/log/?h=wip/logcat I have this warning when the *.lang file is read by GtkSourceView: (lt-test-widget:23351): GtkSourceView-WARNING **: in file /home/seb/jhbuild/share/gtksourceview-4/language-specs/logcat.lang: style 'def:text' not defined Please fix this warning, you should test the *.lang file by opening the application from the terminal, to see if there are some warnings or errors. (I tested with tests/test-widget). Please take the commit from the wip/logcat branch, so that I don't need to redo the modifications again.
Created attachment 361835 [details] [review] Add logcat.lang Hopefully I captured what you wanted in this version (I started with the wip branch you suggested but applied it to the current branch). I opened it in terminal and verified that the error was gone. (Do you know why I can't or how to map-to a global style?) I ended up running check-language.sh and update-pot.sh which subsequently added logcat to the "Other" sections in language-specs.pot If you would prefer that I didn't do this please let me know and I'll upload a patch without it. Thank you again for your suggestions and assistance.
Created attachment 361836 [details] [review] Add logcat.lang Adding the correct file this time.
Review of attachment 361836 [details] [review]: It looks better, thanks. One more small comment. ::: data/language-specs/logcat.lang @@ +30,3 @@ + <style id="comment" name="Comment" map-to="def:comment"/> + <style id="verbose" name="Verbose" map-to="xml:processing-instruction"/> + <style id="debug" name="Debug" map-to="diff:location"/> If possible it's better to map-to styles that are defined in def.lang. "xml:processing-instruction" seems unrelated to "verbose", and "diff:location" seems unrelated to "debug".
Created attachment 361906 [details] [review] Add logcat.lang I picked those because they were universally defined and provide a distinct difference visually between the types. The attached version is sufficient to distinguish them but ideally I would be able to reference an unformatted text type for the verbose. If there is a way to do that please let me know. If not go with the attached patch.
(In reply to scarab96 from comment #8) > but ideally I would be able to reference an unformatted > text type for the verbose. If there is a way to do that please let me know. The map-to attribute of <style> is optional, if it is not present the text with that style will not be highlighted by default, but it is possible to modify a style scheme to apply a style.
I've pushed the patch, when opening the test file it looks good. The *.lang file can of course be improved, if you want to change the map-to's for example. Thanks for your contribution! Attachment 361906 [details] pushed as 0b23e30 - Add logcat.lang