Read-only feature

[andresmmera]

See #763 for more context...

This is a new feature for locking schematics. Screencast here

In the qucsdoc class, it was set two boolean flags: readonly and locked. The first indicates if the file has read-only permissions whereas the latter is a parameter set by the user from the Qucs GUI. In both cases, the user input should be disabled and I think that the proper way to do that is by locking the viewport() and blocking the editDelete signal.

The lock feature only supports schematic files. The reason for that is that we need to modify (somehow) the document to make the lock property permanent. In this sense, I added a tag in the field schematic file format, but this is only included when the schematic is locked. Since the most common case is to work with unlocked schematics, the tag is not added for them in favor of (some) backwards compatibility. Obviously, we cannot proceed this way with other type of documents...

(I posted a screenshot of my text editor since github seems to have trouble with the tag format...)

Finally, a new button was added to the panel so as to let the user to lock/unlock the schematic.

Hope this helps

[felix-salfelder]

thanks, this makes sense.

if (isTextDocument(DocumentTab->currentWidget()))

i suggest to do an assert(pointer); before every dereference operation. at least in new code.

Schematic * d = (Schematic*)DocumentTab->currentWidget();

not good. do

 assert(DocumentTab);
 widget* w = DocumentTab->currentWidget();
 Schematic * d = dynamic_cast<Schematic*>(w);
 assert(d);

instead. (always like this, at least in new code).

[andresmmera]

@felix-salfelder thanks. Once there, I've also corrected existing code concerning the Schematic dereferences.

[in3otd]

I assume this is about When the user opens the schematic, detect if it is read-only and disable changes (see here). I'm not sure that we need to save the locked state in the document itself, I think it should be something that just exists for the current Qucs instance, something similar to the Open read only feature found in many editors. If a document is not writeable it will be opened in read-only mode by default. (OTOH, one day we may implement a file/project locking using lock files where we can put additional info about which Qucs process is using the file/project, which user since when, etc. but that would be another PR)

Besides, the schematics can still be modified when

• doing Select All and Cut or Paste
• using Undo/Redo

and probably also in other ways

BTW, being able to disable schematic modifications may be helpful for #471 also.

Having the Lock command under the Insert menu seems a bit strange - one locks or allows/disallows modifications of a Document/File so it should better go under the File menu. As said above, we should likely have an Open read only and a Toggle read only there.

[andresmmera]

I assume this is about When the user opens the schematic, detect if it is read-only and disable changes (see here).

Yes, that's right. This PR detects if a file is writable or not at the time the user opens it and appends the "[READ-ONLY]" string to the file name https://github.com/Qucs/qucs/pull/769/files#diff-07a46d9c7b31a19eccbb820ca0cd3aa8R1387

The following screenshot shows the result of opening an example file located at /usr/local/share/qucs/examples/ where I have permission 644.

In addition to this, I thought it would be convenient to have a feature to lock schematics. The idea is to somehow protect a schematic against accidental modifications (I reckon that Microsoft Word does something similar).

I'm not sure that we need to save the locked state in the document itself, I think it should be something that just exists for the current Qucs instance

Well, I don't think so. Suppose that you want to share/distribute certain schematic file, but you want it to be protected against accidental edition. I guess there's no way to do that unless you add some extra information in the header (e.g. a locked field).

Besides, the schematics can still be modified when

doing Select All and Cut or Paste
using Undo/Redo

and probably also in other ways

BTW, being able to disable schematic modifications may be helpful for #471 also.

Having the Lock command under the Insert menu seems a bit strange - one locks or allows/disallows modifications of a Document/File so it should better go under the File menu. As said above, we should likely have an Open read only and a Toggle read only there.

I fully agree. I ignore why I put the lock command under the Insert menu... I would swear it was once under the File menu :grin:

Question: Should I put the lock stuff in a separate branch and leave the read-only stuff here?

[felix-salfelder]

On Wed, May 02, 2018 at 10:25:38AM -0700, Andrés Martínez-Mera wrote:

Well, I don't think so. Suppose that you want to share/distribute certain schematic file, but you want it to be protected against accidental edition. I guess there's no way to do that unless you add some extra information in the header (e.g. a locked field).

what happens if the file is simply read-only? wouldn't that protect from modifications?

i think this protection should kick in, once you try to save. attemps to edit should only raise a warning "you are modifying a read-only file".

Question: Should I put the lock stuff in a separate branch and leave the read-only stuff here?

i think read-only (for a file) and lock (for an object) should use the same code.

one question i have is how and when readonly-flags should be propagated and queried up and down the hierarchy... (and this should be only implemented once).

[andresmmera]

what happens if the file is simply read-only? wouldn't that protect from modifications?

Yes, the edition is disabled. Both read-only and locked features disable edition. The difference is that the read-only mode disables edition because of the nature of the file permissions and in the locked mode is the user who disables edition from the Qucs GUI.

i think this protection should kick in, once you try to save. attemps to edit should only raise a warning "you are modifying a read-only file".

Ok, I see. I'll need to rework this PR a little bit.

one question i have is how and when readonly-flags should be propagated and queried up and down the hierarchy...

I didn't think about that... Should the read-only flags propagate through the hierarchy? I guess the right thing is to treat each file individually, isn't it?

[in3otd]

So a Document can be non-modifiable because of the file permissions or because we would like to protect it from accidental editing.

In summary the possibilities should be something like

• a normal (writeable) schematic file
  • can be open read/write (the default Open action)
  • once loaded one can toggle the read-only status but this will only disallow editing in the current Qucs session and not have any effect on the actual file permissions
  • can be open read-only, via a Open read only menu)
  • once loaded one can toggle the read-only status to allow editing
  • under Document Properties there will be a Open read-only (the "lock" feature) field that can be checked/unchecked
  • if the Document has the Open read-only property set it will be open as read-only by default
• a read-only (non writeable) schematic file
  • will be open as read-only
  • read-only status can still be changed in Qucs to allow editing
  • can not be saved with the same name
  • can be saved as a non-read only file with a different name

If you'd like to implement both things now you can put everything in this branch.

Not sure I understood the readonly-flags hierarchical propagation, is this about subcircuits? I think every file (schematic, subcircuit, data display) should have its own read-only flags.

[felix-salfelder]

On Wed, May 02, 2018 at 12:28:38PM -0700, in3otd wrote:

Not sure I understood the readonly-flags hierarchical propagation, is this about subcircuits? I think every file (schematic, subcircuit, data display) should have its own read-only flags.

(i don't see why these three should be different. in gnucap, there is CARD_LIST, and it looks appropriate for the three purposes.)

i argue that the readonly flag should be stored in the container, not within every single component. also, every object (down to ELEMENT?) must know its owner. using the same data type for the container in the cases above will make this easier.

anyway, maybe the hierarchical stuff is relevant in cases where you right click on an instance and go "down to schematic". that's where we should propagate the flag from the owner to the container. i don't know how this is currently implemented in qucs, but it should be straightforward.

[andresmmera]

Hum, it smells like we're opening a can of worms :-)

I guess I'd be better if we stick to the task "When the user opens the schematic, detect if it is read-only and disable changes" (here) and leave apart the lock feature for the moment.

This means:

1. Qucs checks the file permissions at the time the user opens it.
2. In case the file is not writable, Qucs must disable edition: The user can modify the schematic, but he/she cannot save it.
3. The user may save the schematic in some other folder where he/she has write permission.

Do you agree with this?

[in3otd]

Do you agree with this?

YES!

[in3otd]

would also have been nice to have the pointer dynamic cast stuff changes and the actual functional changes in separate commits (git add -p can help with that)

[andresmmera]

Done. I have added an extra commit, 8bd6f0b, since I thought it would be convenient to ask the user to save the document into a different location when he/she tries to save a read-only schematic.

[andresmmera]

I've just corrected the issue you spotted: https://github.com/Qucs/qucs/pull/769/commits/923e81e8947aae0a73de3746c03000b827e485be#diff-2db52a5f64aeeea81fd7b1231486253cR84 It simply checks if the file exist, if not it returns "false".

Indentation corrected. I think it's ok now: https://github.com/Qucs/qucs/pull/769/commits/5277f318093049dea80e2ab9233bc114d9594390#diff-d85a0f8c94a4eee7d0284775e8e30f25R508