pino (Pino Toscano)
User

Projects

User does not belong to any projects.

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Wednesday

  • Clear sailing ahead.

User Details

User Since
Mar 25 2016, 4:10 PM (130 w, 3 d)
Availability
Available

Recent Activity

Yesterday

pino committed R157:e0e78688da8f: appdata: use https for website (authored by pino).
appdata: use https for website
Sun, Sep 23, 9:30 PM
pino committed R157:4c92197e335e: appdata: fix URLs of screenshots (authored by pino).
appdata: fix URLs of screenshots
Sun, Sep 23, 9:30 PM
pino committed R883:1525170: new translation.
new translation
Sun, Sep 23, 6:53 PM
pino requested changes to D15690: Removed old style casts.
Sun, Sep 23, 4:41 PM · KDE Edu
pino added a comment to D15690: Removed old style casts.
  • again, please do not mix unrelated changes, just submit separate and documented review for them
  • please follow a bit more the existing coding style
Sun, Sep 23, 4:41 PM · KDE Edu
pino committed R226:b6ccfae3336f: use QIcon::fromTheme() instead of QIcon() (authored by pino).
use QIcon::fromTheme() instead of QIcon()
Sun, Sep 23, 10:12 AM
pino committed R157:17014f4a897a: i18n: extract messages (authored by pino).
i18n: extract messages
Sun, Sep 23, 8:09 AM
pino committed R157:d9c1306ab69f: i18n: set the application domain (authored by pino).
i18n: set the application domain
Sun, Sep 23, 8:09 AM
pino committed R157:95b650613a5d: i18n fixes (authored by pino).
i18n fixes
Sun, Sep 23, 8:09 AM
pino requested changes to D15690: Removed old style casts.

This patch does more things that "remove old style casts". Some of the other changes are OK, but please do split them in own changes, properly documenting them.

Sun, Sep 23, 6:14 AM · KDE Edu

Sat, Sep 22

pino added a comment to D15692: Add POT export and PO import..
In D15692#330282, @pino wrote:

Not sure I understand, what is the exact workflow that involves writing a .pot file, and reading .po files? Is it because peruse so far is not translatable?
If so, then it'd be much better to extract the messages, and make sure to use ki18n to translate the strings at runtime, just like all the other applications.

Ah, no, what this is for is something else. You see, Peruse Creator makes cbz(comic book) files, and also generates metadata files for cbz, this is done in the ACBF format. The ACBF format supports metadata for text within the comic and holding translations for said text. I figured that it'd be helpful to be able to take these translatable entries, and generate POT files for them (as well as import of translation po files). That way people who'd want to translate a comic can use their favourite pot/po compatible translation tools.

Sat, Sep 22, 8:02 PM · Peruse
pino added a comment to D15692: Add POT export and PO import..

Not sure I understand, what is the exact workflow that involves writing a .pot file, and reading .po files? Is it because peruse so far is not translatable?
If so, then it'd be much better to extract the messages, and make sure to use ki18n to translate the strings at runtime, just like all the other applications.

Sat, Sep 22, 7:46 PM · Peruse
pino committed R883:1525081: updates.
updates
Sat, Sep 22, 9:25 AM

Wed, Sep 19

pino added a comment to D15605: kdev-astyle : upgrade libastyle to v3.1.

Normally I'd agree, but here we're dealing with really a tiny amount of additional code and a project which isn't exactly being kept up-to-date by distributions either (on Linux I'd get 2.04 from the system, on Mac 2.05 from MacPorts).

Wed, Sep 19, 9:01 PM · KDevelop
pino added inline comments to D15532: [Astyle] Add Objective C to list of languages with formatters.
Wed, Sep 19, 6:41 PM · KDevelop
pino added a comment to D15605: kdev-astyle : upgrade libastyle to v3.1.

What about using the system version of libastyle, instead? I.e. look for libastyle, and if not found then do not build kdev-astyle.
This would avoid a code copy which clearly is not kept up-to-date, with almost no overhead on packagers (since astyle provides a shared library for a long time).
Since astyle 3.0 broke the ABI and changed its API a bit, using that version as minimum could be a reasonable idea.

Wed, Sep 19, 6:38 PM · KDevelop

Tue, Sep 18

pino added a comment to D15093: Add WireGuard capability..
In D15093#327917, @pino wrote:

note there are still few "not done" comments around (eg using QSpinBox for fwMark)

Yes I know, I'll get to the spinbox in the next day or so. I've only been putting it off because I think it is such a bad idea and I hate making something I wrote worse rather than better but I know that I am playing in your house so I need to play by your rules. I'll hold my nose and get to it soon.

Tue, Sep 18, 9:33 PM · Plasma
pino committed R108:9d49ba6a97a9: fixuifiles (authored by pino).
fixuifiles
Tue, Sep 18, 5:52 AM
pino requested changes to D15093: Add WireGuard capability..

note there are still few "not done" comments around (eg using QSpinBox for fwMark)

Tue, Sep 18, 5:41 AM · Plasma
pino added a comment to D15520: Upgrade SimpleIpV4AddressValidator and SimpleIpV6AddressValidator.
  • please remove all the empty initTestCase() in tests
Tue, Sep 18, 5:17 AM · Plasma
pino removed a dependency for D15093: Add WireGuard capability.: D15521: Add validator for lists of IP addresses Added as separate review per comment from Pino on review D15093. This code will not compile without the updated code in review D15520. Also includes unit test..
Tue, Sep 18, 5:15 AM · Plasma
pino removed a dependent revision for D15521: Add validator for lists of IP addresses Added as separate review per comment from Pino on review D15093. This code will not compile without the updated code in review D15520. Also includes unit test.: D15093: Add WireGuard capability..
Tue, Sep 18, 5:15 AM · Plasma

Mon, Sep 17

pino requested changes to D15520: Upgrade SimpleIpV4AddressValidator and SimpleIpV6AddressValidator.
  • same notes for simpleipv6test.cpp as in simpleipv4test.cpp
Mon, Sep 17, 6:23 AM · Plasma
pino added dependencies for D15093: Add WireGuard capability.: D15520: Upgrade SimpleIpV4AddressValidator and SimpleIpV6AddressValidator, D15521: Add validator for lists of IP addresses Added as separate review per comment from Pino on review D15093. This code will not compile without the updated code in review D15520. Also includes unit test..
Mon, Sep 17, 6:22 AM · Plasma
pino added a dependent revision for D15520: Upgrade SimpleIpV4AddressValidator and SimpleIpV6AddressValidator: D15093: Add WireGuard capability..
Mon, Sep 17, 6:22 AM · Plasma
pino added a dependent revision for D15521: Add validator for lists of IP addresses Added as separate review per comment from Pino on review D15093. This code will not compile without the updated code in review D15520. Also includes unit test.: D15093: Add WireGuard capability..
Mon, Sep 17, 6:22 AM · Plasma
pino requested changes to D15521: Add validator for lists of IP addresses Added as separate review per comment from Pino on review D15093. This code will not compile without the updated code in review D15520. Also includes unit test..
Mon, Sep 17, 6:15 AM · Plasma
pino added inline comments to D15140: Fix random order in "Analyze Current File/Project With" menus.
Mon, Sep 17, 6:01 AM · KDevelop

Wed, Sep 12

pino requested changes to D15093: Add WireGuard capability..

A couple of questions on opening other review requests:

  • Should I open a bug request on bugs.kde.org before opening the review?
Wed, Sep 12, 7:08 AM · Plasma
pino requested changes to D15440: Translated into Russian.

NACK, this is not the correct approach.
The right approach is translate the messages via the .po file that contains them: https://websvn.kde.org/trunk/l10n-kf5/ru/messages/kde-workspace/plasma-browser-extension.po?view=markup (and the same in branches/stable/l10n-kf5). There is an automatic system that extracts the messages into plasma-browser-extension.pot, and injects all the translations back to the repository in form of messages.json files.

Wed, Sep 12, 4:52 AM · Plasma

Sun, Sep 9

pino requested changes to D15093: Add WireGuard capability..

Thanks Bruce for all the updates, and patience! In return, I have more notes :)

  • I'd send the changes to the existing SimpleIpV4AddressValidator in an own review request, since it affects other code than just this new wireguard plugin (maybe with unit tests)
  • I'd send the addition of SimpleIpListValidator in an own review request too, since it is an independent code (possibly with unit tests)
  • something I forgot (sorry) in the past: QRegExp is the "old" class for regexps, while in new code QRegularExpression should be preferred/used; should be mostly a transparent change for your uses
  • I notes various nits for the UI strings: this is to follow the HIG, see https://hig.kde.org/style/writing/index.html in particular
  • if possible, please avoid HTML markup in strings in UI files -- they make translations slightly more complicated (e.g. more prone to break the string with a typo in the markup)
Sun, Sep 9, 10:10 PM · Plasma
pino committed R883:1524462: remove obsolete docs & move existing.
remove obsolete docs & move existing
Sun, Sep 9, 6:32 AM
pino committed R883:1524459: remove obsolete docs.
remove obsolete docs
Sun, Sep 9, 6:15 AM
pino committed R883:1524458: remove obsolete docs.
remove obsolete docs
Sun, Sep 9, 6:13 AM
pino committed R883:1524457: remove obsolete docs.
remove obsolete docs
Sun, Sep 9, 6:10 AM
pino committed R883:1524456: remove obsolete docs, and duplicated english images.
remove obsolete docs, and duplicated english images
Sun, Sep 9, 6:07 AM
pino committed R883:1524454: remove obsolete docs.
remove obsolete docs
Sun, Sep 9, 5:53 AM
pino committed R883:1524453: remove english image.
remove english image
Sun, Sep 9, 5:47 AM
pino committed R883:1524452: remove obsolete docs.
remove obsolete docs
Sun, Sep 9, 5:46 AM
pino committed R883:1524450: remove obsolete docs, and duplicated english images.
remove obsolete docs, and duplicated english images
Sun, Sep 9, 5:40 AM
pino committed R883:1524448: remove duplicated/obsolete english images.
remove duplicated/obsolete english images
Sun, Sep 9, 5:34 AM
pino committed R883:1524447: remove obsolete files and docs.
remove obsolete files and docs
Sun, Sep 9, 5:31 AM

Sat, Sep 8

pino committed R883:1524346: remove one more obsolete doc.
remove one more obsolete doc
Sat, Sep 8, 8:39 PM
pino committed R883:1524345: remove old docs.
remove old docs
Sat, Sep 8, 8:36 PM
pino committed R883:1524343: remove old doc.
remove old doc
Sat, Sep 8, 4:23 PM
pino committed R103:431cfc8afd07: cmake: look for DocTools & Init frameworks (authored by pino).
cmake: look for DocTools & Init frameworks
Sat, Sep 8, 12:32 PM
pino accepted D15345: Mark phonon as required.

Please commit it in the Applications/18.08 branch.

Sat, Sep 8, 10:07 AM · KDE Edu
pino committed R37:c40245270974: fixuifiles (authored by pino).
fixuifiles
Sat, Sep 8, 4:51 AM
pino committed R37:27c96dc13520: fixuifiles (authored by pino).
fixuifiles
Sat, Sep 8, 4:50 AM

Tue, Sep 4

pino requested changes to D14932: l10n fixes.

I stopped at Viewer/ViewerWidget.cpp, but the main point is: please do not add whitespace/indentation changes to this patch. This patch is already big because of all the i18n changes, no need to make it way bigger by adding unrelated changes; they make it harder to review what was the actual topic of the commit.

Tue, Sep 4, 4:40 AM

Mon, Sep 3

pino added a comment to D15093: Add WireGuard capability..

If you as a representative of the plasma-nm philosophy have a preference on which way to go or have a brilliant idea which solves all the problems, I will follow your lead.

Mon, Sep 3, 5:23 PM · Plasma
pino requested changes to D15093: Add WireGuard capability..

Much better now!

Mon, Sep 3, 6:01 AM · Plasma

Sun, Sep 2

pino committed R883:1523931: fix json keys.
fix json keys
Sun, Sep 2, 2:52 PM

Fri, Aug 31

pino resigned from D15173: Add a new ISO 3166 code to country name mapping table.

We could make iso-codes a hard build-time requirement and generate this file every time, to address this concern. The CI has it on all platforms but Windows already it seems. Would that be acceptable?

Fri, Aug 31, 8:03 AM · KDE PIM
pino added a comment to D15173: Add a new ISO 3166 code to country name mapping table.

I don't see how loading it once per process would be a big issue, compared to embedding data in the library.
Also, the proposed approach here creates another kind of static data in the sources (isotocountrymap_data.cpp); considering it is not automatically generated, this means that it will be hardly updated by anyone, and happily rot in the sources. Just look at the existing countrytransl.map, for example.

Fri, Aug 31, 7:30 AM · KDE PIM
pino requested changes to D15176: l10n fixes.

NACK, duplicate of D14932: l10n fixes

Fri, Aug 31, 6:38 AM
pino added a comment to D14932: l10n fixes.

I can not find how to send new changes with arc to Phabricator...

Fri, Aug 31, 6:38 AM
pino requested changes to D15173: Add a new ISO 3166 code to country name mapping table.
Fri, Aug 31, 6:36 AM · KDE PIM
pino added a comment to D15173: Add a new ISO 3166 code to country name mapping table.

Please lookup the strings at runtime, instead of embedding them in the library.
Otherwise, an update to iso-codes (like updating the name of a country) will break the mapping, since the installed translations will not reflect the old text embedded.

Fri, Aug 31, 6:36 AM · KDE PIM
pino added a comment to D15093: Add WireGuard capability..

I've used KDE for years but this is the first time I've written code using Qt so it doesn't surprise me that I didn't use some of the preferred methods of doing things. I have a few questions below and hopefully you will have a little patience with me if any seem like stupid questions.

In D15093#315890, @pino wrote:

Misc notes:

  • please remove all the translations (i.e. Name=[lang] & Comment[lang] from desktop/json files, since KDE has an automatic system to handle them

Do the automatic translations get added into the desktop files themselves at some point of the build process?

Fri, Aug 31, 5:03 AM · Plasma

Thu, Aug 30

pino committed R162:f1b191ba3b6a: i18n style fixes (authored by pino).
i18n style fixes
Thu, Aug 30, 4:41 AM

Wed, Aug 29

pino committed R162:fb16edd46904: i18n fixes (authored by pino).
i18n fixes
Wed, Aug 29, 6:21 AM

Tue, Aug 28

pino added a comment to D15132: Fix build with libraw 0.19.

I'd say to use this version, and merge it to master.
As noted in D15133: Drop functionality that depends on libraw features removed in 0.19, this approach IMHO is way safer, and less disruptive for users of libkdcraw (and downstreams too...).

Tue, Aug 28, 9:55 PM
pino requested changes to D15133: Drop functionality that depends on libraw features removed in 0.19.

No need to break the API/ABI of libkdcraw just because of changes in libraw -- just mark the public stuff affected as deprecated.
Regarding the internal usage: please keep the old code for older versions of libraw; see the libraw_version.h header (provided by libraw) with some defines & macros for this.

Tue, Aug 28, 9:52 PM
pino committed R883:1523544: update and complete.
update and complete
Tue, Aug 28, 4:34 AM

Mon, Aug 27

pino added inline comments to D15024: Update icons kcm docbook to 5.13.
Mon, Aug 27, 6:05 PM · Documentation, Plasma
pino added a comment to D15093: Add WireGuard capability..

As far as the 'passwordfield' files, this was just laziness on my part, it being easier to just copy them than figure out how to get cmake (another tool I haven't used much) to add the correct include path to the flags. Any pointers on how to do this correctly would be appreciated.

Mon, Aug 27, 6:28 AM · Plasma
pino committed R883:1523483: update.
update
Mon, Aug 27, 5:57 AM
pino requested changes to D15093: Add WireGuard capability..

Misc notes:

  • please remove all the translations (i.e. Name=[lang] & Comment[lang] from desktop/json files, since KDE has an automatic system to handle them
  • please use KConfig to read & write ini-like files, instead of doing everything manually
  • the PasswordField class is already available as libs/editor/widgets/passwordfield.h, part of the plasmanm_editor library, so you do not need to copy it locally
  • wireguard_global.h seems not used, so just drop it
  • nm-wireguard-service.h has lots of NM_OPENVPN_ keys that are not used
  • as @lbeltrame said, please use QHostAddress to parse IP addresses
  • as @lbeltrame said, hardcoding colors is a bad idea, and it will not play nice with user choice of different color schemes, or accessibility purposes
  • a better option to validate an input in a line edit is to use the embedded validator options, see QLineEdit::setInputMask and QValidator
  • the better option to edit a port number is QSpinBox restricted to 0-65536, instead of a line edit
  • there is a mixture of virtual & Q_DECL_OVERRIDE for virtual methods; plasma-nm uses override exclusively
  • please do not hardcode sizes and fonts in UI files
  • there are various texts in UI files with double spaces between words, please simply them to a single space
  • manually calling KAcceleratorManager::manage(this); is not needed, why were they added?
Mon, Aug 27, 5:42 AM · Plasma

Sun, Aug 26

pino requested changes to D14932: l10n fixes.

Mostly good now!

Sun, Aug 26, 9:32 PM

Sat, Aug 25

pino added a comment to D15083: Use title case for the clipboard button.

For the fix itself: you do not need a diff review for this kind of changes, especially for master branches that were recently opened again for development...

Sat, Aug 25, 5:39 PM · Documentation
pino added a comment to D15083: Use title case for the clipboard button.
In D15083#315441, @pino wrote:

Question for the localization folks: can I change this string in the stable branch?

There is the string freeze in stable branches.

So I guess that's my question: is the freeze a total freeze, or does changing capitalization not count?

Sat, Aug 25, 5:33 PM · Documentation
pino added a comment to D15083: Use title case for the clipboard button.

Question for the localization folks: can I change this string in the stable branch?

Sat, Aug 25, 5:26 PM · Documentation

Aug 25 2018

pino committed R883:1523281: update.
update
Aug 25 2018, 1:04 PM

Aug 22 2018

pino requested changes to D14932: l10n fixes.

Note that all the i18n() calls where you add a context that was not there before (say i18n("foo") -> i18n("@title:window", "foo")) they will not build, since i18n() does not take a context. You must switch them to i18nc() instead.
This, plus the other note I left, makes me think that this was not build-tested... please make sure that this change at least builds fine.

Aug 22 2018, 4:39 AM

Aug 21 2018

pino added inline comments to D14962: Fix build with julia 1.0.
Aug 21 2018, 4:58 AM · Cantor, KDE Edu
pino requested changes to D14939: Making more recommended xplanet changes and fixing bugs:.
Aug 21 2018, 4:56 AM · KDE Edu

Aug 20 2018

pino requested changes to D14939: Making more recommended xplanet changes and fixing bugs:.

I have more notes, but no time right now.

Aug 20 2018, 6:04 AM · KDE Edu
pino added a comment to D14288: Initial version of Clazy analyzer plugin.

FWIW: Okular uses the Discount library for reading markdown documents, showing them as HTML. Also Cantor will do so too: D14738: Add the markdown entry.

Maybe it'd be a good idea to use it -- there are few options:

  • make the library a hard dependency for this plugin (which then would not be built if discount is not found
  • optionally use discount, using this simple fallback otherwise (though with the risk of rotting)
  • optionally use discount, and show no documentation if not found

I think about this library before writing current custom parser. It looks good and easy to use but adds extra dependency, especially for AppImage/Windows installer.

Aug 20 2018, 5:44 AM · KDevelop
pino updated the summary of D14119: Add screenshot to gdrive.appdata.xml..
Aug 20 2018, 4:53 AM
pino added inline comments to D14119: Add screenshot to gdrive.appdata.xml..
Aug 20 2018, 4:47 AM
pino requested changes to D14932: l10n fixes.

Getting better :)

Aug 20 2018, 4:36 AM
pino updated subscribers of D14932: l10n fixes.
Aug 20 2018, 4:25 AM
pino requested changes to D14937: Add screenshot to gdrive.appdata.xml..

Duplicated of D14119: Add screenshot to gdrive.appdata.xml..

Aug 20 2018, 4:24 AM
pino added a comment to D14119: Add screenshot to gdrive.appdata.xml..
In D14119#311645, @sten wrote:

I tried arc diff --update 14119 (and other numeric revision-like numbers in this thread) to try to update it, without success.

Aug 20 2018, 4:23 AM

Aug 19 2018

pino committed R883:1522965: typo fix.
typo fix
Aug 19 2018, 9:04 PM
pino committed R883:1522963: updates.
updates
Aug 19 2018, 8:36 PM
pino added inline comments to D14931: Eliminate duplicate QMaps in OutputWidget.
Aug 19 2018, 8:19 PM · KDevelop
pino added a comment to D14932: l10n fixes.

Hi Antoni,

Aug 19 2018, 8:10 PM
pino added inline comments to D14288: Initial version of Clazy analyzer plugin.
Aug 19 2018, 1:31 PM · KDevelop
pino committed R262:c4439e63b7db: fixuifiles (authored by pino).
fixuifiles
Aug 19 2018, 5:39 AM

Aug 18 2018

pino added a comment to D14813: Adding XPlanet Viewer.

I can still make more changes to the code if needed. Many of the things you mention though that are left are not actually my doing due to the fact that it was already that way in the existing code before I started. If they need to be changed in this code, they should be changed elsewhere as well. I did these things the way I thought we were already doing them.

Aug 18 2018, 7:46 PM · KDE Edu
pino committed R468:b12a9475797a: Use KMessageWidget in PostscriptDialog (authored by pino).
Use KMessageWidget in PostscriptDialog
Aug 18 2018, 3:55 PM
pino committed R468:25d96119b45e: no-op open&save .ui files (authored by pino).
no-op open&save .ui files
Aug 18 2018, 3:55 PM
pino committed R325:17f530c34879: i18n fix: avoid contractions (authored by pino).
i18n fix: avoid contractions
Aug 18 2018, 1:50 PM
pino committed R883:1522914: updates.
updates
Aug 18 2018, 1:49 PM
pino committed R456:e49d799eeba6: i18n fixes (authored by pino).
i18n fixes
Aug 18 2018, 1:32 PM
pino added a comment to D14813: Adding XPlanet Viewer.

FWIW, there are still lots of my concerns that remain unfixed -- sad to see that this patch was committed like this.

Aug 18 2018, 7:10 AM · KDE Edu

Aug 17 2018

pino committed R216:9f9c9af0eba5: debchangelog: add Bookworm (authored by pino).
debchangelog: add Bookworm
Aug 17 2018, 4:42 AM

Aug 16 2018

pino added a comment to D13712: Simplify page size configuration.

ping?

Aug 16 2018, 5:27 AM · KBibTeX