the-carlisle-group / Acre-Desktop

A simple Dyalog APL IDE plugin that introduces "projects" and allows you to keep your source code in Unicode text files.
MIT License
11 stars 1 forks source link

Problem with SetChanged #211

Closed aplteam closed 4 years ago

aplteam commented 4 years ago

The ]acre.SetChanged #.NamespaceName -recursive syntax has, AFAIC, a problem:

It saves everything in #.NamespaceName and its sub-namespaces, including variables, no matter what the general setting for tracking variables is.

I think this is wrong. By default SetChange should honour the general setting, which defaults to not tracking variables.

However, SetChange should offer an option -vars=on|off which defaults to the general setting. If one wants it to act differently that option can be used.

PhilLast commented 4 years ago

From the documentation:

SetChanged is one of the two ways to circumvent the bar on storing variables in a project when its Variables configuration parameter is "Off". Once saved further edit changes will continue to be saved.

(The other is EditArray)

This adds a new dimension to the problem in that SetChanged is the direct analogue and precursor of LINK.Add which will add anything and it the only way to store variables. Kai argues that storing variables within a namespace that is being added is a different thing and should be governed by the "Variables": "On"|"Off" option.

It seems to me that adding an already populated namespace to an existing project is something that a user is likely to take care about, including making sure nothing is in it that he/she does not want. ]Erase can always be used to rid the project of anything that gets in there accidentally. Also I think it will make the command much more difficult to understand.

Nevertheless Kai has much more experience of actually using acre on multiple projects that I have and I'm willing to bow to that if no-one else objects.

Except that the override he proposes ought to be the same - i.e. -variables - or we could add -vars as a synonym or replacement - and it probably shouldn't be necessary when a variable is specified per se rather than as an unspecified member of a space.

aplteam commented 4 years ago

Okay, I judged that from a kind of theoretical perspective. I admit that from a practical view it's perfectly fine to write everything, including variables.

I never saved anything that came with any variable at all with the notable exception of a Laguntza help system, when, at least at the start, everything is about variables. In either case your sugesstion would work just fine: suggestion withdrawn.

However, let's leave this open because at the moment SetChanged is supposed to save variables, but it's not actually doing this with -recursive.

PaulMansour commented 4 years ago

What is the use-case for set changed not being recursive when applied to a namespace?

aplteam commented 4 years ago

Norbert argued that specifying just a namespace name is too dangerous, and brought up the idea of requiring .* after the name in order to indicate that it's really the whole damn thing that should be processed. Now .* is gone and replaced by -recursive.

This is less about SetChanged and more about Erase, naturally, but the syntax is exactly the same.

aplteam commented 4 years ago

By design