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

adding marks for using lightweight markups such as txt2tags #2

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

farvardin
Copy link

See some screenshots there to view the new additions:
http://wiki.txt2tags.org/index.php/Main/Geany

@codebrainz
Copy link
Member

This commit only updates 8 of 27 colour schemes and it's unclear whether you have actually tested these to see if there are any errors (looking in Debug Messages or running geany -v).

If I'm not confident you have tested these with various languages and checked everything is sane, and since the change adds almost as many new styles as there are existing ones, this means that in order for me to merge your changes will probably take me like a week of solid boring work just to merge changes that I'm not even sure I like as is (although I agree support for light markup languages definitively needs improvement).

As it stands, I'm a little concerned about (in no particular order):

  • Lack of testing
  • Not following conventions (not so big deal I have a scripts that will fix most of it)
  • Too many very specific new named styles Actually looking closer at the specific styles, they seem generic enough to cover most light markup languages.
  • Missing 19 of the 27 color schemes
  • Duplication of existing styles (ex. preprocessor)
  • I'm not sure about the markup_ prefix on all of the styles, but maybe there's a reason I can't think of. See: lightweight markup colorscheme geany#255 (comment) (although I'm not sure I agree this is needed or useful).
  • Need to update existing filetypes (ex. markdown) to make use of the new named styles (although this will be a separate PR on geany/geany). See: lightweight markup colorscheme geany#255

@codebrainz
Copy link
Member

Here's an example of what I was hoping for using mc.conf arbitrarily, and note that I absolutely did not test this (requires testing against geany/geany changes, etc, too much work for prototype).

codebrainz/geany-themes@1b62107

codebrainz/geany-themes@286753a

BTW, don't worry about making Git busywork, just push more commits to this PR and don't worry about clobbering my branch either. Once it's ready to go I'll squash/rebase/cherry-pick as needed for final merge.

@farvardin
Copy link
Author

About testing the additions to the schemes I don't know how to do this. I hope it won't take too much of your time for mergin this. I thought it would be quite trivial because the new marks are different from the existing ones, so how could this interfere with the rest of the geany code?

What I did is this:

  • I've build geany on my computer and installed it using "make install".
  • I've loaded a couple of file and everything looked fine (I've openened some js, css and html files for example). Unless there are some regression tests I don't know how to test this better.

For the missing color schemes, if you agree with what I did so far, I'll integrate the new marks in them as well.

About the colors, I've most of the time used geany or gcolor2 to pick and modify them, and they only give color codes with 6 digits.

@codebrainz
Copy link
Member

Cool it's looking better. What do you think about my commits mentioned above, mainly removing the markup_ prefix and removing comment, preproc and postproc. For preproc you could map the filetypes.* styles to existing preprocessor named style and for postproc you could map it to something like other. If postproc and preproc are available in more markups than txt2tags, I'm OK for adding them if you really want to, it's just nice to not have very language specific styles in the schemes.

@codebrainz
Copy link
Member

Oh, also for testing, just run Geany from the terminal like geany -v and then open some files for testing and if there are errors, it will output some debug messages like Bad color or stuff like this.

@farvardin
Copy link
Author

With geany -v I get those errors when opening a css file:

Geany-INFO: Bad color 'element'
Geany-INFO: Bad color 'pseudoclass'
Geany-INFO: Bad color 'pseudoelement'

but no color related error when parsing txt2tags files. I still get "geany: Warning: ignoring null tag in ..." but it's an old bug I guess and it's not related to the new color scheme (I got this for years but it doesn't bother me)

About removing markup_ I don't really have any idea, I first made this (= some years ago) by following the names used in markdown and adding some more, then recently b4n suggested to add markup_ to avoid conflicts, and now you suggest to remove it again :)

I think it could be sensitive to separate lightweight markups styles and coding styles but you can choose what you prefer. It's just I already made some txt2tags color schemes for other editors such as kate or scite and I wanted them to be consistent (see for example http://wiki.txt2tags.org/index.php/Main/Scite ). To be honest I don't think there are other languages which has a preprocessor and a postprocessor, so you decide. Btw I don't need that many style names, I just need a few colors, like in the solarized them, it was enough with yellow, magenta, cyan, blue etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants