Delete point without confirmation / configurable

Issue #235 resolved
Felix Ulber created an issue

When deleting a point from your pattern, it is always asked to confirm that. This is really slowing down work. From several different programs I know it as a good thing that I can set an option like "don't ask me again" (I know what I am doing and I have an undo function :) ).

This then has to be adjustable in the preferences.

Comments (15)

  1. Felix Ulber reporter

    This looks good - so it is not from the main api but from the Qt Creator source code? Can one just use it like that (I mean in terms of license)? On a quick glance I see it is directly usable with QSettings - I think valentina settings use this mechanism?

  2. Roman Telezhynskyi repo owner

    This looks good - so it is not from the main api but from the Qt Creator source code? Can one just use it like that (I mean in terms of license)?

    How i know Qt Creator allow use code under LGPL license.

    On a quick glance I see it is directly usable with QSettings - I think valentina settings use this mechanism?

    Yes, see VSettings class.

  3. Felix Ulber reporter

    Just got it working quickly hacked in. A few questions arise though:

    • Naming of the VSettings property and where it should go. I quickly named it like AskForPointDeletion and would create a new section on the first page of the Preferences Dialog (did not yet). I also could imagine an additional preferences page with all advanced/not so important settings in a simple property/value list. What do you think?

    • Where should the CheckableMessageBox source files go? I put them in app/dialogs/app for now

    • The new message box is called in VAbstractTool::ConfirmDeletion(), replacing the standard QMessageBox. This is nice, but raises two more questions:

    1. The dialog now directly takes the according settings key (Ask for delete?) and cares about. So either that one askForDeletion setting could apply to all derived tools, or you have to know which derived tool class is actually calling ConfirmDeletion to hand over the correct settings key which is closely related to
    2. the CheckableMessageBox takes the string name of the settings' key, which is ok to keep it universal. Unfortunately the VSettings class hides all the settings keys names in private members and has everything done by interface (getter/setter), keeping me from directly handing over a certain setting by name to the message box call (temporaily using the string literal atm). What is the intention with that (besides having the ability to have some advanced getter/setter that handle e.g. unit conversion)? One could handle all that manually in ConfirmDeletion, but this makes it more complicated, rewriting some of the stuff that CheckableMessageBox is already capable of.

    Sorry, lots of stuff here, I hope not to make things overcomplicated.. :)

  4. Roman Telezhynskyi repo owner

    I have what to answer, but it is too late now for clear mind. Need go to rest.

  5. Roman Telezhynskyi repo owner

    This class looks good, but not so good how i thought if use static function.

    Why just don't use class instead of static function? And check with bool isChecked() const user answer.

    Also better don't mix two licenses LGPL and GPL. Better use exist (VPropertyExplorer library) or create new static library for this purpose.

    void VToolDetail::DeleteTool(bool ask)
    

    bool ask we use for another purpose, and most time it is true.

    I see this issue much easier. 1. Take class CheckableMessageBox. 2. Create settings key. 3. In method DeleteTool check if need to show message inside of

    if (ask)
    {
    //...
    }
    
    1. After return check if user ask don't show anymore delete questions.
  6. Felix Ulber reporter

    Why just don't use class instead of static function? And check with bool isChecked() const user answer. I see this issue much easier. 1. Take class CheckableMessageBox. 2. Create settings key. 3. In method DeleteTool check if need to show message inside of if (ask){//...} After return check if user ask don't show anymore delete questions.

    Hm, I wanted to avoid something like that for I don't find it very elegant. Anyway. Stll don't understand completey if u want to use one setting for all delete tools or be able to separate.

    What's that VPropertyExplorer thing? Unfortunately it is not documented. What is that existing ask parameter actually for? (no problem, I did not touch it anyhow) What is about the preferences window?

  7. Roman Telezhynskyi repo owner

    **Stll don't understand completey if u want to use one setting for all delete tools or be able to separate.
    One for all.

    What's that VPropertyExplorer thing? Unfortunately it is not documented.
    It is library that keep classes that show property of tools.

    What is that existing ask parameter actually for? (no problem, I did not touch it anyhow)
    Tool union details delete old details and create new one. This tool use this parameter.

    What is about the preferences window?
    I like page Pattern. It half empty now.

  8. Felix Ulber reporter

    What's that VPropertyExplorer thing? Unfortunately it is not documented. It is library that keep classes that show property of tools.

    Sorry, one last question on that hopefully :) What do you actually propose to use it for in this case (or instead of what)?

  9. Roman Telezhynskyi repo owner

    I propose for now save this class in library VPropertyExplorer even this class not for working with property.

  10. Felix Ulber reporter

    Hey Roman, just completed this thing - just before I commit it there is one question: What do you think how the license text in the checkablemessagebox files should be modified? That is something I have no experience with.

  11. Roman Telezhynskyi repo owner

    Did you take code from Qt Creator source code? If yes, you can't change license. I am not sure if mix license good idea. And also don't want mix in one binary files with several licenses. I propose you for now save class in library vpropertyexplorer. This library has LGPL license. We also can add new library, maybe even static. But one new class it is not very big reason for creation library. And also library vpropertyexplorer use only Valentina so we can do what we want.

  12. Felix Ulber reporter

    I propose you for now save class in library vpropertyexplorer.

    Thats what I did now, I am just asking how to handle - cause some little changes were made to the source code. If you have a look at the comment on top of the checkablemessagebox source file - can we just leave it and add something like "modified..."?

  13. Log in to comment