model.writer#setMarker should not change marker's options when they are not set

[scofalik]

When creating a marker you can set whether the marker is managed through operations:

const marker = modelWriter.setMarker( 'myMarker', myRange, { usingOperations: true } );

However, at the moment, the method works in a way, that you need to pass that object every time because when not set, the option will be overwritten:

modelWriter.setMarker( marker, myOtherRange ); // Will set `usingOperations` to `false`.

Instead, when options object is not set, setMarker call should not change any options. Only this should change an option:

modelWriter.setMarker( marker, myOtherRange, { usingOperations: false } );

[pjasiun]

Writing this comment I realised that it would be inconsistent: setSelection will reset selection options but setMarker will not.

This is why I think we should split this method and have separate changeMarker/updateMaker.

const marker = setMarker( [ name = uid() ], range, [ options ] );

updateMarker( markerOrName, [ range ], [ options ] )

Set marker should not accept "marker" as the first parameter. It could create a new marker instance and should return marker instance. It removes old maker if the marker with such name existed.

Update marker should not create new marker instance but only update the existing instance. It should return nothing. It should change only options you define.

[pjasiun]

To make it safer we should make option.usingOperation obligated in setMarker, so it needs to be defined every time you use this method. It is too easy to make a mistake now.

[jodator]

@pjasiun does option.usingOperation should be obligatory for updateMarker()?

[pjasiun]

@pjasiun does option.usingOperation should be obligatory for updateMarker()?

No. The whole point of updateMarker is that it will do not change range or option if you do not set it.

[jodator]

@pjasiun just wanted to be sure.

Anyway second question:

writer.setMarker( [ name ], range, options );
writer.updateMarker( markerOrName, range, [ options ] )

I have problem with the first parameter (name) being optional in setMarker() and options object being required.

Optional parameters should go as last but it looks weir a bit now.

writer.setMarker( range, { using: true } );
writer.setMarker( range, { using: true }, 'myMarker' );

writer.updateMarker( 'myMarker', range );
writer.updateMarker( 'myMarker', range, { using: true } );

Or maybe:

writer.setMarker( range, 'myMarker', { using: false } );
writer.setMarker( range, { using: true } );


writer.updateMarker( range, 'myMarker' );
writer.updateMarker( range, myMarker, { using: true } );

I can see at least two sensible options:

1. Make options.usingOperation optional in setMarker().
2. Make setMarker( range, options, name ) and updateMarker( name, range, options ).

[jodator]

After talks with @pjasiun and that we cannot come to a cleaner solution will make name param required for writer#setMarker() as:

• we do not have usages for empty name
• developer can call uid() and pass it as a name

[pjasiun]

Yep. Also, we agreed to move range to options.