archimatetool / archi-modelrepository-plugin

coArchi - a plug-in to share and collaborate on Archi models.
153 stars 53 forks source link

The built-in save action in Archi should trigger a save #9

Closed jbsarrodie closed 7 years ago

jbsarrodie commented 7 years ago

The built-in save action in Archi should trigger a save (whatever it means) if the model has been opened with this plugin.

Phillipus commented 7 years ago

IEditorModelManager#addPropertyChangeListener(this)

   public void propertyChange(PropertyChangeEvent evt) {
            String propertyName = evt.getPropertyName();
            if(propertyName == IEditorModelManager.PROPERTY_MODEL_SAVED) {
                haveABeer();
            }
    }
jbsarrodie commented 7 years ago

From a UX point of view, I think user should be able to save his work through the usual keyboard shortcut without having to interract with any dialog. So this means that the (Archi built-in) save action should not create a commit.

The save action included in this plugin could open a dialog asking the user if he wants to create a commit. The UI could be a simple/unique dialog with a checkbox (unchecked by default) that let user decide to add a commit, and a text area (enabled only if checkbox checked) to enter the message.

What follows has been written before your last comment. I keep it here in cas we need it : I thought about it and I think a simple (but effective) approach would be to use the .archimate file created in the .git folder. We only have to be able to detect if user changed it or not, and this can be done easily if we calculate a hash of its content and save the relsult in another small file in the .git folder. And when a commit is created, we just have to force a save and calculate a new hash. So in the future, we'll be able to check if content has been saved but not commited (e.g. when switching to another branch)

Phillipus commented 7 years ago

The save action included in this plugin could open a dialog asking the user if he wants to create a commit.

So, it's still the existing commit action but we might call it something else?

Phillipus commented 7 years ago

The save action could still trigger a Grafico export.

Phillipus commented 7 years ago

Then it seems to me:

@jbsarrodie what is the intent of a checkbox?

Phillipus commented 7 years ago

At the moment the temp.archimate model is getting out of sync with the grafico xml files. We need to decide if an Archi Save does a Grafico export at the same time. Or else when the user opens the model, the app, or another action we need to sync the temp.archimate file. We have to use the temp.archimate file because Archi only knows about *.archimate files when it re-opens them in the tree.

jbsarrodie commented 7 years ago

Let's try to clarify....

I suggest that we do our best to always have the "temp.archimate" file and the working tree in sync. This means that saving the file has to trigger a Grafico export, and creating a commit has to save the file.

That being said, this leads me to:

So there's no more Save action as part of the plugin (only Archi Save action)

My only (minor) concern is: how to explain "commit" to a user ?

Phillipus commented 7 years ago

My only (minor) concern is: how to explain "commit" to a user?

A "commit" represents a change set. It marks a significant point of change in the model such as "Created new View" or "Added core model elements for use case xxxx". Thus a "Commit" could be renamed to something like: Change Point, Milestone, Mark, etc...

Phillipus commented 7 years ago

And we have to account for when the user works on the temp file with the plugin closed (not listening to save events) Therefore needs to sync on plugin opening.

jbsarrodie commented 7 years ago

And we have to account for when the user works on the temp file with the plugin closed (not listening to save events) Therefore needs to sync on plugin opening.

Agree but can we do that ? (at least for the moment that's not a big issue as the working tree will be in sync as soon as the user will save or commit)

Phillipus commented 7 years ago

There are Archi "open" actions that load the temp file:

And in the plugin there are actions that open the model from the Grafico importer using the git xml files:

jbsarrodie commented 7 years ago

My only (minor) concern is: how to explain "commit" to a user?

A "commit" represents a change set. It marks a significant point of change in the model such as "Created new View" or "Added core model elements for use case xxxx". Thus a "Commit" could be renamed to something like: Change Point, Milestone, Mark, etc...

I've just read several howto (in french) and found the root of the (now solved) issue. In english, we use commit as both a noon and a verb, but this is not possible in french. So the best way (at least in french) is to use:

Which leads me to the action "Valider les changements". Not sure the translation "validate changes" works great in english though.

jbsarrodie commented 7 years ago

And in the plugin there are actions that open the model from the Grafico importer using the git xml files:

  • Open
  • Refresh
  • Clone

I agree, the only question on my side is "how easy is it to trigger a Grafico export when loading the model in Archi without be too intrusive?"

If that's easy then great. If not, then (as a workaround)

Phillipus commented 7 years ago

I was thinking about what "temp.archimate" is bringing to the party and wondering how to avoid it. But I don't think we can avoid it:

So we can't really bypass this by opening the model directly from the Grafico files, unless we add all of Archi's save/open/load code to Grafico or add hooks.

Phillipus commented 7 years ago

Not sure the translation "validate changes" works great in english though.

No, not really. ;-)

"Commit changes" is OK.

jbsarrodie commented 7 years ago

Having thought a little bit more....

When restarting Archi. Whot do we consider the most valid ?

So we can't really bypass this by opening the model directly from the Grafico files, unless we add all of Archi's save/open/load code to Grafico or add hooks.

I agree, so we might consider that the temp.archimate file is the "official" working tree, and that the git working tree is used only when absolutly needed.

If we do, then the answer to my question at the begining is "a changed temp.archimate file"

jbsarrodie commented 7 years ago

Not sure the translation "validate changes" works great in english though.

No, not really. ;-)

So we might stick to "Commit changes" in english and "Valider les changements" en french...

Phillipus commented 7 years ago

So we might stick to "Commit changes" in english and "Valider les changements" en french...

Parfait. Pour l'instant...

Phillipus commented 7 years ago

Save is implemented. But See Issue #13

jbsarrodie commented 7 years ago

Ok, closing this one