ECSTATE_SAVED / ECSTATE_MODIFIED changed their meaning

[johnd0e]

Far Manager version

6460

OS version

10

Other software

No response

Steps to reproduce

1. Open a file in the editor, make modifications, and save it. Check editor.GetInfo().CurState - the ECSTATE_MODIFIED flag is not set.
2. Open the editor using Shift+F4, providing an empty filename. Check editor.GetInfo().CurState - the ECSTATE_SAVED flag is set, even though the file does not yet exist.

Expected behavior

1. If a file was modified during an edit session, it used to retain the ECSTATE_MODIFIED flag even after being saved. Perhaps this was unintentionally changed in build 6185, see issue #700. @rohitab @alabuzhev
2. I'm not sure how it behaved previously, but the current behavior seems illogical.

In either case, the current behavior is incorrect. It's now only possible for a file to be either "modified" or "saved"; it's no longer possible for both states to be unset. If this is the intended correction, then why do we need two flags when one could serve the same purpose?

Actual behavior

[yegor-mialyk]

Let me share my "amateur" opinion on ECSTATE_MODIFIED and ECSTATE_SAVED, as I've never used those directly.

In my view, the current behavior is correct because it ensures that a file's state is always clear: it is either marked as modified (when there are unsaved changes) or marked as saved (when the buffer matches the last saved version).

So, while I understand the concern about past behavior, I personally think the current approach is logical and aligns with behaviors in other text editors and IDEs.

In fact, like you said before, since the ECSTATE_MODIFIED and ECSTATE_SAVED states are mutually exclusive today, only a single flag - such as ECSTATE_MODIFIED - would actually be enough to capture all the necessary information about the file's status.

Looks like ECSTATE_SAVED is needed only for backward compatibility.

[johnd0e]

it is either marked as modified (when there are unsaved changes) or marked as saved (when the buffer matches the last saved version).

• It was also useful to know if file was ever modified in current edit session. I actually used that: imagine scenario when plugin presents to user some config to edit, and then (on editor close) needs to find out if it was actually edited or not.
• When some file is new (and still empty) - it is really neither saved, nor modified. If we for any reason would decide to implement different meaning - that should be documented clear and bold.

[rohitab]

@johnd0e Apologies for the late reply.

It was also useful to know if file was ever modified in current edit session.

I agree. In fact, this information is already available via other places. For example, Editor.State returns this information as seen in the following code:

https://github.com/FarGroup/FarManager/blob/96d48b7ffc9fef530857655384174e8f7a758dcd/far/fileedit.cpp#L884

It should be possible to return this same information via ECTL_GETINFO. Something like the following should work.

--- a/far/editor.cpp
+++ b/far/editor.cpp
@@ -5613,7 +5613,10 @@ int Editor::EditorControl(int Command, intptr_t Param1, void *Param2)
 			Info->BookmarkCount=BOOKMARK_COUNT;
 			Info->SessionBookmarkCount=GetSessionBookmarks(nullptr);
 			Info->CurState=m_Flags.Check(FEDITOR_LOCKMODE)?ECSTATE_LOCKED:0;
-			Info->CurState|=!m_Flags.Check(FEDITOR_MODIFIED)?ECSTATE_SAVED:0;
+			if (const auto HostFileEditor = GetHostFileEditor())
+			{
+				Info->CurState |= HostFileEditor->WasFileSaved()?ECSTATE_SAVED:0;
+			}
 			Info->CurState|=m_Flags.Check(FEDITOR_MODIFIED)?ECSTATE_MODIFIED:0;
 			Info->CodePage = GetCodePage();
 

[johnd0e]

It should be possible to return this same information via ECTL_GETINFO. Something like the following should work.

@alabuzhev ?

[alabuzhev]

ok, changed as proposed.

[nightroman]

Just to let you know, changes broke some of my established editor saving flows and logic.

I'll investigate and maybe adapt but these are breaking changes (not quite right, fellows).

[nightroman]

The enc. is not helpful. It just lists flag names and says that combinations are possible.

To be honest, I am not sure what the new functionally is, could somebody explain please? What flags and combinations actually mean now?

[rohitab]

@nightroman Sorry to hear you ran into issues with this change.

To be honest, I am not sure what the new functionally is, could somebody explain please?

Please see @johnd0e explanation above.

What flags and combinations actually mean now?

My understanding is this:

ECSTATE_MODIFIED

File has been modified, i.e., there are unsaved changes. The editor status bar should show an asterisk * indicator.

ECSTATE_SAVED

The file was saved in this session. This flag is independent of ECSTATE_MODIFIED. If the user saves the file, then modifies it, both ECSTATE_SAVED and ECSTATE_MODIFIED will be set. Note that when a file is first opened, both flags are unset, since the file has not been saved or modified.

Before Far version 6562, these flags were related. , i.e., ECSTATE_SAVED was the logical not of ECSTATE_MODIFIED. If you need the old functionality, you should be able to use ECSTATE_MODIFIED to get both states.

[nightroman]

@rohitab Thank you for the details. I went with using just ECSTATE_MODIFIED which translates to FarNet API IEditor.IsModified. As for ECSTATE_SAVED (IsSaved), I deprecate it in FarNet and will remove soon, it is confusing next to IsModified, imho.

[alabuzhev]

it is confusing next to IsModified, imho

ECSTATE_SAVED is basically "the user touched the file". It could be useful in scenarios where you present something to the user as a temporary file. E.g. you have a UserMenu-like thing in your plugin and want to let the user edit it as text, similar to Alt+F4 in the real UserMenu. If the flag is set, the user made some changes and thus the file should be parsed and some internal structures should be updated after exiting the editor. If the flag is not set, all that can be skipped, because the user only looked at it.

[johnd0e]

ECSTATE_MODIFIED

File has been modified, i.e., there are unsaved changes. The editor status bar should show an asterisk * indicator.

ECSTATE_SAVED

The file was saved in this session.

That is behavior after 6562. It covers all the cases, so I'm happy with it. But! Before 6185 the flags had opposite meaning: ECSTATE_SAVED was "has no unsaved changes", and ECSTATE_MODIFIED was "ever modified in current session".

I'm personally fine with both variants. But current implementation means in fact "breaking changes". That can be bad for old unsupported plugins.

[nightroman]

@johnd0e That's what I was trying to tell. The change effectively changes nothing, just breaks things (my things included). The problems in plugins might not be noticed right away, depends on plugin usage / testing.

The enc. should have properly described the original flags long ago. Then maybe desire for the change would not happen.

FWIW I am fine if this change is undone.

[alabuzhev]

Ugh. If we just switch the flags, would they be more or less compatible with their historic meaning?

[alabuzhev]

ECSTATE_SAVED was "has unsaved changes"

This doesn't make much sense. Did you mean to say "has NO unsaved changes"?

[johnd0e]

This doesn't make much sense. Did you mean to say "has NO unsaved changes"?

Yes, exactly.

If we just switch the flags, would they be more or less compatible with their historic meaning?

I believe so. And to check it you need just to get Far <6185 (I will do that now).

[johnd0e]

I believe so. And to check it you need just to get Far <6185 (I will do that now).

Macro {
  key="F1"; area="Editor";
  action=function()
    local ei = editor.GetInfo()
    local out = {}
    for k,v in pairs(far.Flags) do
      if k:match"^ECSTATE_" and band(ei.CurState, v)~=0 then
        table.insert(out, k)
      end
    end
    far.Message(table.concat(out," + "))
  end;
}

https://github.com/FarGroup/FarManager/releases?q=6184&expanded=true

1. Just opened editor (incl. empty new file): ECSTATE_SAVED
2. Modified: ECSTATE_MODIFIED
3. Saved: ECSTATE_MODIFIED + ECSTATE_SAVED

[shmuz]

I've just last days finished changes in my codes to adapt to the new behavior.

Saved: ECSTATE_MODIFIED + ECSTATE_SAVED

This would make me to revert all my corrections.

[alabuzhev]

Just opened editor (incl. empty new file): ECSTATE_SAVED

Makes sense if we say that "saved" means "no unsaved changes".

Modified: ECSTATE_MODIFIED

Does not make sense, the underlying file is not modified yet.

Saved: ECSTATE_MODIFIED + ECSTATE_SAVED

Makes sense: no unsaved changes and the underlying file is modified.

---

I've tried to flip the flags like this:

diff --git a/far/editor.cpp b/far/editor.cpp
index 27a270b7f..d36a307a0 100644
--- a/far/editor.cpp
+++ b/far/editor.cpp
@@ -5605,8 +5605,8 @@ int Editor::EditorControl(int Command, intptr_t Param1, void *Param2)
 			Info->SessionBookmarkCount=GetSessionBookmarks(nullptr);
 			Info->CurState=m_Flags.Check(FEDITOR_LOCKMODE)?ECSTATE_LOCKED:0;
 			if (const auto HostFileEditor = GetHostFileEditor())
-				Info->CurState |= HostFileEditor->WasFileSaved()? ECSTATE_SAVED : 0;
-			Info->CurState|=m_Flags.Check(FEDITOR_MODIFIED)?ECSTATE_MODIFIED:0;
+				Info->CurState |= HostFileEditor->WasFileSaved()? ECSTATE_MODIFIED : 0;
+			Info->CurState |= m_Flags.Check(FEDITOR_MODIFIED)? 0 : ECSTATE_SAVED;
 			Info->CodePage = GetCodePage();
 
 			return true;

And got the following results:

1. Just opened editor (incl. empty new file): ECSTATE_SAVED ✅ Matches pre-6185 behavior.
2. Modified: no flags ❌ Does not match pre-6185 behavior, but see above - it seems to be wrong anyway.
3. Saved: ECSTATE_MODIFIED + ECSTATE_SAVED ✅ Matches pre-6185 behavior.

Are we happy with such a mapping?

[nightroman]

Why gamble and not just revert to the old and document properly? Apparently all plugins somehow adapted to the old, without much complains.

[johnd0e]

Why gamble and not just revert to the old and document properly?

It's hard to document anything that makes no sense - see case 2.

Are we happy with such a mapping?

I am happy, and old plugins should be 95% happy.

So meaning should be:

• ECSTATE_SAVED - has no unsaved changes
• ECSTATE_MODIFIED - was saved in current session

[nightroman]

You should also explain what absence of flags means.

[johnd0e]

• no ECSTATE_SAVED - has unsaved changes
• no ECSTATE_MODIFIED - was not yet saved in current session

[shmuz]

It's hard to document anything that makes no sense - see case 2.

No, to document something one must not explain why it is that way.

[johnd0e]

No, to document something one must not explain why it is that way.

You you would like explanation like this:

• ECSTATE_MODIFIED - was saved in current session OR has unsaved changes
• no ECSTATE_MODIFIED - was not saved in current session AND has no unsaved changes

P.S. I'm rather fine with this too, but... Isn't it just nonsense?

[shmuz]

Personally for me it does not matter how it will be in the end. I do not use old 3-rd party plugins. Do any variant and finish with it.

[nightroman]

Personally for me it does not matter how it will be in the end. I do not use old 3-rd party plugins. Do any variant and finish with it.

Same here.

[alabuzhev]

Isn't it just nonsense?

If we reword it as "ECSTATE_MODIFIED means that either the underlying file or the editor content has been modified" it should be easier to comprehend, but it still doesn't make much sense and I can't yet think of a scenario where this would matter.

@johnd0e, what about the legacy macro api, Editor.State bits 3 and 6? Do they still work the same as before or do they also need adjustments?

[alabuzhev]

"ECSTATE_MODIFIED means that either the underlying file or the editor content has been modified"

Interestingly, this is exactly how it was defined initially:

https://github.com/FarGroup/FarManager/blob/fca0451a4037789580645df3cc6924678949b219/far/editor.cpp#L5016-L5017

The || Modified in ECSTATE_MODIFIED is obviously redundant, since it's fully inferable from the absence of ECSTATE_SAVED.

So, shall we restore this quirk just in case or have more straightforward logic?

[johnd0e]

what about the legacy macro api, Editor.State bits 3 and 6?

In general bit 3 corresponds to "not SAVED", and bit 6 to "MODIFIED".

Macro {
  key="F1"; area="Editor";
  action=function()
    local state = Editor.State
    local out = {}
    if band(state, 2^3)==0 then table.insert(out, "SAVED") end
    if band(state, 2^6)~=0 then table.insert(out, "MODIFIED") end
    far.Message(table.concat(out," + "))
  end;
}

BTW, in pre-6185 it worked without quirks, so:

So, shall we restore this quirk

No, better without quirks.

Do they still work the same as before or do they also need adjustments?

There is difference when file was opened and F2 pressed without modifications.

• pre-6185 - bit 6: clear
• now - bit 6: set

Previous behavior makes more sense for me. And it was in sync with ECSTATE_MODIFIED.

[alabuzhev]

Previous behavior makes more sense for me

I do not agree. You pressed F2 and the file has been entirely and destructively overwritten. The fact that the new content happen to exactly match the old content is just a coincidence.

No, better without quirks.

Ok, let's stay quirk-free for now. We can always add it later if needed. 6570.