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

Draw spaces, tabs, newlines and nbsp #412

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@kreijack
Copy link

commented Mar 23, 2019

This pull request allows to draw spaces, tabs, newlines and nbsp in Pluma. This capability is useful when you want to control if spaces or tab are used.

This pull request consists of two commits: the first one allow to control the drawing of these entities via gsettings:

  • spaces (org.mate.pluma.enable-space-drawer-space)
  • tabs (org.mate.pluma.enable-space-drawer-tab)
  • newlines (org.mate.pluma.enable-space-drawer-newline)
  • not blank space (org.mate.pluma.enable-space-drawer-nbsp)

The second commits allow to (un)set these options in the preferences dialog ('Editor' tab)
sc

Please merge.

BR
G.Baroncelli kreijack@inwind.it

@lukefromdc lukefromdc requested a review from mate-desktop/core-team Mar 24, 2019

@raveit65

This comment has been minimized.

Copy link
Member

commented Mar 31, 2019

This branch cannot be rebased due to conflicts !
Edit:
There are old commits from other people commits in this PR.
You should try to rebase (not merge) against current master branch.

@kreijack kreijack force-pushed the kreijack:visible-nl branch from 6caf19e to 73b4ebb Apr 1, 2019

@kreijack

This comment has been minimized.

Copy link
Author

commented Apr 1, 2019

Hi,

I rebased my branch to master; please re-consider to apply this pull-request.

BR
G.Baroncelli

@raveit65

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

Does this looks nice?
Your commit in browser https://patch-diff.githubusercontent.com/raw/mate-desktop/pluma/pull/412.patch

Can you please fix intends in a lot of places?
And we prefer spaces instead of tabs in new written code.
Tabs are making only problems with indents in different editors with different tab sizes.

And can you avoid abbreviations in description of gsettings key, please?

+    <key name="enable-space-drawer-nbsp" type="b">
+      <default>false</default>
+      <summary>Draw nbsp</summary>
+      <description>Whether pluma should draw nbsp.</description>

Nobody will understand this.....

@kreijack

This comment has been minimized.

Copy link
Author

commented Apr 9, 2019

Does this looks nice?
Your commit in browser https://patch-diff.githubusercontent.com/raw/mate-desktop/pluma/pull/412.patch

Can you please fix intends in a lot of places?

What does you means with "intends" ? Did you mean "indents" ?

And we prefer spaces instead of tabs in new written code.
OK; I update the patch following the rules below:

  • if a completely new function (declaration) is added, I used spaces instead of tabs
  • otherwise I followed the style of the code above/below (basically tabs everywhere)

Anyway I adjusted the indentation in the patch (unfortunately sometime I wrote with a tab size of 4 chars, this caused the mis-alignements ...)

Tabs are making only problems with indents in different editors with different tab sizes.

And can you avoid abbreviations in description of gsettings key, please?

+    <key name="enable-space-drawer-nbsp" type="b">
+      <default>false</default>
+      <summary>Draw nbsp</summary>
+      <description>Whether pluma should draw nbsp.</description>

Nobody will understand this.....
NBSP is a well used acronyms. Anyway I expanded that in 'Not Breaking SPace'.

@kreijack kreijack force-pushed the kreijack:visible-nl branch from 73b4ebb to fadb920 Apr 9, 2019

@vkareh
Copy link
Member

left a comment

This is quite wonderful. I'm honestly a huge fan of visible spaces and tabs (but I usually don't show the newline character).

Maybe out of scope for this, but adding trailing spaces at the of each line would be fantastic. We have a plugin that automatically removes them on save, but it could be nice to be able to see them.

Anyway, my "change request" is a small change in the copy for &nbsp; and the indentation on this change, it looks a bit unkempt. Also, I'm not entirely sure how to test the nbsp toggle. There's no checkbox in the UI for it, and now sure how to type one...

Other than that, this built well and without errors and works really well. Thanks! 👍

Show resolved Hide resolved pluma/pluma-prefs-manager-app.c
Show resolved Hide resolved pluma/pluma-prefs-manager-app.c
Show resolved Hide resolved pluma/pluma-prefs-manager-app.c Outdated
Show resolved Hide resolved data/org.mate.pluma.gschema.xml.in Outdated
@kreijack

This comment has been minimized.

Copy link
Author

commented Apr 11, 2019

Maybe out of scope for this, but adding trailing spaces at the of each line would be fantastic. We have a plugin that automatically removes them on save, but it could be nice to be able to see them.

There is a way to accomplish to this. The latest version of gtk_source_view allow to draw the trailing space only. I.e something like the image below (note the last portion of line 1494):

Untitled

(this is an hand craft images ).

This would change the dialog a bit: i.e. we need a list-box where it is possible to select:

  • draw spaces everywhere
  • draw trailing spaces only
  • no drawing spaces at all

What do you think ?

@SoongNoonien

This comment has been minimized.

Copy link

commented Apr 19, 2019

First of all I realy miss this feature in Pluma and I hope it will be merged soon.
To answer your question, I would prefer something hierarchical with checkboxes:

  • draw spaces (everywhere)
    • only trailing ones

This would be a little shorter.

@sc0w

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

warnings in this PR:

pluma-prefs-manager.h:126: Warning: Pluma: symbol='GPM_SPACE_DRAWER_SPACE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_SPACE'
pluma-prefs-manager.h:127: Warning: Pluma: symbol='GPM_SPACE_DRAWER_TAB': Unknown namespace for symbol 'GPM_SPACE_DRAWER_TAB'
pluma-prefs-manager.h:128: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NEWLINE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NEWLINE'
pluma-prefs-manager.h:129: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NBSP': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NBSP'
pluma-prefs-manager-app.c:1503:25: warning: ‘gtk_source_view_get_draw_spaces’ is deprecated: Use 'gtk_source_space_drawer_get_types_for_locations' instead [-Wdeprecated-declarations]
                         value = gtk_source_view_get_draw_spaces (GTK_SOURCE_VIEW (l->data));
                         ^~~~~

pluma-prefs-manager-app.c:1508:25: warning: ‘gtk_source_view_set_draw_spaces’ is deprecated: Use 'gtk_source_space_drawer_set_types_for_locations' instead [-Wdeprecated-declarations]
                         gtk_source_view_set_draw_spaces (GTK_SOURCE_VIEW (l->data),
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pluma-view.c:433:9: warning: ‘gtk_source_view_set_draw_spaces’ is deprecated: Use 'gtk_source_space_drawer_set_types_for_locations' instead [-Wdeprecated-declarations]
         gtk_source_view_set_draw_spaces(GTK_SOURCE_VIEW (view),
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

There is no replacement to the deprecated functions?

@sc0w

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

it seems GtkSourceSpaceDrawer is the replacement:

https://developer.gnome.org/gtksourceview/unstable/GtkSourceSpaceDrawer.html

@kreijack

This comment has been minimized.

Copy link
Author

commented May 1, 2019

Maybe out of scope for this, but adding trailing spaces at the of each line would be fantastic. We have a plugin that automatically removes them on save, but it could be nice to be able to see them.

There is a way to accomplish to this. The latest version of gtk_source_view allow to draw the trailing space only. I.e something like the image below (note the last portion of line 1494):

Untitled

(this is an hand craft images ).

This would change the dialog a bit: i.e. we need a list-box where it is possible to select:

* draw spaces everywhere

* draw trailing spaces only

* no drawing spaces at all

What do you think ?

I like the idea. However the "space-draver" interface appeared only the gtksourceview >= 3.24 (date about 2017). However pluma requires gtksourceview >= 3.0. I think that the 3.24 revision is too recent to be a strong requirement. So I am working to support both the interface:

  • if gtksourceview < 3.24, there is the possibility to draw all or none 'spaces/tab...' (
  • if gtksourceview => 3.24, there is the possibility to draw all, the trailing or none 'spaces/tab...'

It would require a bit more working, but it is doable.

@kreijack

This comment has been minimized.

Copy link
Author

commented May 1, 2019

First of all I realy miss this feature in Pluma and I hope it will be merged soon.
To answer your question, I would prefer something hierarchical with checkboxes:

* draw spaces (everywhere)
  
  * only trailing ones

This would be a little shorter.

Yes, it makes sense.

@kreijack

This comment has been minimized.

Copy link
Author

commented May 1, 2019

it seems GtkSourceSpaceDrawer is the replacement:

https://developer.gnome.org/gtksourceview/unstable/GtkSourceSpaceDrawer.html

Only for gtksourceview >= 3.24. I am working on that, however I have to implement a fallback solution if the code is not available.

@kreijack

This comment has been minimized.

Copy link
Author

commented May 1, 2019

warnings in this PR:

pluma-prefs-manager.h:126: Warning: Pluma: symbol='GPM_SPACE_DRAWER_SPACE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_SPACE'
pluma-prefs-manager.h:127: Warning: Pluma: symbol='GPM_SPACE_DRAWER_TAB': Unknown namespace for symbol 'GPM_SPACE_DRAWER_TAB'
pluma-prefs-manager.h:128: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NEWLINE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NEWLINE'
pluma-prefs-manager.h:129: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NBSP': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NBSP'

I saw that. However I found that this is true also for other symbols....

pluma-prefs-manager.h:115: Warning: Pluma: symbol='GPM_LOCKDOWN_SCHEMA': Unknown namespace for symbol 'GPM_LOCKDOWN_SCHEMA'
pluma-prefs-manager.h:116: Warning: Pluma: symbol='GPM_LOCKDOWN_COMMAND_LINE': Unknown namespace for symbol 'GPM_LOCKDOWN_COMMAND_LINE'
pluma-prefs-manager.h:117: Warning: Pluma: symbol='GPM_LOCKDOWN_PRINTING': Unknown namespace for symbol 'GPM_LOCKDOWN_PRINTING'
pluma-prefs-manager.h:118: Warning: Pluma: symbol='GPM_LOCKDOWN_PRINT_SETUP': Unknown namespace for symbol 'GPM_LOCKDOWN_PRINT_SETUP'
pluma-prefs-manager.h:119: Warning: Pluma: symbol='GPM_LOCKDOWN_SAVE_TO_DISK': Unknown namespace for symbol 'GPM_LOCKDOWN_SAVE_TO_DISK'
pluma-prefs-manager.h:122: Warning: Pluma: symbol='GPM_DEFAULT_AUTO_SAVE_INTERVAL': Unknown namespace for symbol 'GPM_DEFAULT_AUTO_SAVE_INTERVAL'
pluma-prefs-manager.h:123: Warning: Pluma: symbol='GPM_DEFAULT_MAX_RECENTS': Unknown namespace for symbol 'GPM_DEFAULT_MAX_RECENTS'
pluma-prefs-manager.h:126: Warning: Pluma: symbol='GPM_SPACE_DRAWER_SPACE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_SPACE'
pluma-prefs-manager.h:127: Warning: Pluma: symbol='GPM_SPACE_DRAWER_TAB': Unknown namespace for symbol 'GPM_SPACE_DRAWER_TAB'
pluma-prefs-manager.h:128: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NEWLINE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NEWLINE'
pluma-prefs-manager.h:129: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NBSP': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NBSP'
@raveit65

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Any news or progress?
Or should we close PR?

@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 7, 2019

Draw spaces, tabs, newlines and nbsp.
Via gsettings it is possible to enable the drawing of:
- newline (enable-space-drawer-newline)
- spaces (enable-space-drawer-space)
- tab (enable-space-drawer-tab)
- not blank space (enable-space-drawer-nbsp)

The first setting is a boolean ones, so the only allowable values
are true (show the symbol) or false (don't show the symbol).
Instead for the last three settings, the allowed values are:
'draw-none' -> show nothing
'draw-all' -> show all spaces/tabs
'draw-trailing' -> show only the trailing spaces/tabs (only if GTK >= 3.24)

@kreijack kreijack closed this Jun 15, 2019

@kreijack kreijack force-pushed the kreijack:visible-nl branch from fadb920 to ce90454 Jun 15, 2019

@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 15, 2019

I update my pr: it is possible to reopen it ?
s
I update the code in order to use the GtkSourceSpaceDrawer , so it would be possible to enable/disable the [trailing] spaces/tabs/newlines in any combination. I had to put some
#ifdef GTK_SOURCE_VERSION_3_24
in order to be compatible with gtk < 3.24 which doesn't have this interface. Unfortunately the work is not yet finished because I had to implement the double interface (after/before 3.24) at the ui level.

Let me know if it is possible to reopen this pr or I had to reopen a new one

@raveit65

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

hmm, reopen-button is still grey-out for me, that's weird

@raveit65

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

But you should be able to open a new one.

@raveit65

This comment has been minimized.

Copy link
Member

commented Jun 15, 2019

Looks like that there are no commits?


kreijack:visible-nl was force-pushed and no longer has any new commits.

Pushing new commits will allow the pull request to be re-opened.
@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 16, 2019

Now I update the branch.

@sc0w sc0w reopened this Jun 16, 2019

@sc0w

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

@kreijack

thanks to continue the work of this PR

Seeing the logs now with latest changes, I see:

[pluma/pluma-prefs-manager-app.c:1489]: (style) Unused variable: i

pluma-prefs-manager-app.c: In function 'pluma_prefs_manager_space_drawer_generic':
pluma-prefs-manager-app.c:1489:15: warning: unused variable 'i' [-Wunused-variable]
         gint  i;
               ^
pluma-prefs-manager.h:126: Warning: Pluma: symbol='GPM_SPACE_DRAWER_SPACE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_SPACE'
pluma-prefs-manager.h:127: Warning: Pluma: symbol='GPM_SPACE_DRAWER_TAB': Unknown namespace for symbol 'GPM_SPACE_DRAWER_TAB'
pluma-prefs-manager.h:128: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NEWLINE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NEWLINE'
pluma-prefs-manager.h:129: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NBSP': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NBSP'

@kreijack kreijack force-pushed the kreijack:visible-nl branch from 772c779 to c2daf78 Jun 21, 2019

@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

@kreijack

thanks to continue the work of this PR

Seeing the logs now with latest changes, I see:

[pluma/pluma-prefs-manager-app.c:1489]: (style) Unused variable: i

pluma-prefs-manager-app.c: In function 'pluma_prefs_manager_space_drawer_generic':
pluma-prefs-manager-app.c:1489:15: warning: unused variable 'i' [-Wunused-variable]
         gint  i;
               ^

Ok, I correct this

pluma-prefs-manager.h:126: Warning: Pluma: symbol='GPM_SPACE_DRAWER_SPACE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_SPACE'
pluma-prefs-manager.h:127: Warning: Pluma: symbol='GPM_SPACE_DRAWER_TAB': Unknown namespace for symbol 'GPM_SPACE_DRAWER_TAB'
pluma-prefs-manager.h:128: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NEWLINE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NEWLINE'
pluma-prefs-manager.h:129: Warning: Pluma: symbol='GPM_SPACE_DRAWER_NBSP': Unknown namespace for symbol 'GPM_SPACE_DRAWER_NBSP'

This was highlighted already. Unfortunately this issue is note related to my change: if you look at the log, you can see:

[...]
pluma-prefs-manager.h:119: Warning: Pluma: symbol='GPM_LOCKDOWN_SAVE_TO_DISK': Unknown namespace for symbol 'GPM_LOCKDOWN_SAVE_TO_DISK'
pluma-prefs-manager.h:122: Warning: Pluma: symbol='GPM_DEFAULT_AUTO_SAVE_INTERVAL': Unknown namespace for symbol 'GPM_DEFAULT_AUTO_SAVE_INTERVAL'
pluma-prefs-manager.h:123: Warning: Pluma: symbol='GPM_DEFAULT_MAX_RECENTS': Unknown namespace for symbol 'GPM_DEFAULT_MAX_RECENTS'
pluma-prefs-manager.h:126: Warning: Pluma: symbol='GPM_SPACE_DRAWER_SPACE': Unknown namespace for symbol 'GPM_SPACE_DRAWER_SPACE'
pluma-prefs-manager.h:127: Warning: Pluma: symbol='GPM_SPACE_DRAWER_TAB': Unknown namespace for symbol 'GPM_SPACE_DRAWER_TAB'
[...]

You can notice that some (all ?) other symbols have the same issue

@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

Dear all,
I update the PR in order to satisfy the last sc0w requirement. I tested the code, and I am confident that it works quite well. Now the patch allows Pluma to show the newlines, tabs and spaces with the appropriate symbols. For the latter two symbols it is possible to show only the trailing tabs/spaces when the GTK version is greater or equal to 3.24. If the GTK version is less than 3.24, the trailing tabs/spaces options are disabled (the other options still remain available).

@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 22, 2019

This is quite wonderful. I'm honestly a huge fan of visible spaces and tabs (but I usually don't show the newline character).

Maybe out of scope for this, but adding trailing spaces at the of each line would be fantastic. We have a plugin that automatically removes them on save, but it could be nice to be able to see them.

Anyway, my "change request" is a small change in the copy for &nbsp; and the indentation on this change, it looks a bit unkempt. Also, I'm not entirely sure how to test the nbsp toggle. There's no checkbox in the UI for it, and now sure how to type one...

Other than that, this built well and without errors and works really well. Thanks! +1

@kreijack kreijack closed this Jun 22, 2019

@kreijack kreijack reopened this Jun 22, 2019

@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 22, 2019

Also, I'm not entirely sure how to test the nbsp toggle. There's no checkbox in the UI for it,
and now sure how to type one...

If you want to test the "nbsp" aspect, you have to create a utf8 file with the U+00a0 character (nbsp). Then you may request to pluma to draw it changing the gsetting value of enable-space-drawer-nbsp (0: no drawing; 1: drawing it everywhere; 2: drawing only the trailing nbsp).

I think that it is a quite setting that it is not necessary that the checkbox is in the pluma preferences dialog. For the other needing, the gsetting value is enough.

@sc0w sc0w requested a review from mate-desktop/core-team Jun 22, 2019

@vkareh
Copy link
Member

left a comment

Other than a few comments regarding using enums, this code looks very good and works well for me!

Show resolved Hide resolved data/org.mate.pluma.gschema.xml.in Outdated
Show resolved Hide resolved pluma/dialogs/pluma-preferences-dialog.c Outdated

@vkareh vkareh requested a review from mate-desktop/core-team Jun 24, 2019

Add draw spaces/tabs/newlines options
Add
- draw spaces
- draw tabs
- draw newlines
options in preference dialog.
If the GTK version is greather or equal to 3.24, it is possible to show
only the trailing tabs/spaces symbols.

@kreijack kreijack force-pushed the kreijack:visible-nl branch from c2daf78 to 540ab29 Jun 24, 2019

@kreijack

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

I update also the preferences dialog:
dialog
The 'Draw trailing....' checkboxes are moved a bit to left, so the hierarchy is better understantable .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.