Development build #200210230100 of NetBeans 4.0
Windows 2000 with FCS build #21 of JDK 1.4.1
I can't tell since which build this occurs but
it's fact that auto-editing feature does not work.
No question pops up when trying to alter a
Steps to reproduce:
1. Invoke "Versioning|Mount Version
2. Select "Empty (Windows NT/2000)" profile, push
"Next >" and then "Edit Commands..." buttons.
3. Expand "Empty" folder and select "Lock" command.
4. Change its "Name" property to "EDIT", push "OK"
and "Finish" buttons.
5. Invoke "Customize" on the filesystem, switch to
"Advanced" tab, set "Really ?" as "Message" and
6. Create new file under the filesystem and turn
on its read-only attribute externally.
7. Open that file into editor and push Enter.
8. No "Really ?" question pops up.
This is a regression. I'm going to explore why it stopped working...
Probably some "optimalization" was made??
Sorry but I need to know what is the problem with text package.
I tried it with Empty Unix profile. No "Really?" message popped out,
that's true. But there were also reverted the typed chars whats the
editors job. Please specify what is wrong.
One more question. Why this really strange thing is marked as P2?
I would mark it P5.
I had big problems even repeat the steps. I guess no user could do
something like that -> changing property name from LOCK to EDIT, thus
activating some components in tab.
It's P2, because it's a regression in functionality.
The problem is, that when the user writes anything to the editor,
AbstractFileSystem.Info.lock() should be called and it's IOException
should be taken into account. This does not work anymore for read-only
Here is the stack trace, that is called for RW files:
The problem is in
org.openide.text.DataEditorSupport$Env.markModified(). You've made the
change on Jul 15.
And I would like to explain why the procedure is so long. It is pretty
easy to reproduce for VSS users but I am afraid most of people don't
have this tool installed. Therefore I provided universal procedure ...
There is no guarranty that editor support will take lock on read-only
file. In fact it was wrong and it is not necessary. The file is not
going to be changed so why to take the lock? The change fixed bigger
problems than is the yours.
Anyway your feature relied on such a flow. And that's incorrect. You
cannot request on the impls to keep any obscure thing alive. There is
no way I could be aware about it.
If you need some functionality, you have to request some new API if
O.K., then I need a new API for this purpose ;-)
I understand, that you can know, that if the file is read-only, it can
not be modified. But you can not be sure, that if file is RW it can be
modified. Thus you need to run lock() anyway. You use 2 methods for
Also when you do not call lock(), you effectively removed a
possibility to revert the file from read-only state to read-write.
Thus I need either:
1) specify in the openide docs, that FileObject.lock() MUST be called
whenever there is an attempt to modify a file and it's IOException (or
UserQuestionException) taken into account, OR
2) create a new API (something like FileObject.canModify(), that would
return true/false (and probably also throw UserQuestionException ??)).
Reassigning to Yarda to make the architectural decission.
If the file can be modified, then it is not read only. Let the VCSFS
not return true from isReadOnly if one can get output stream and write
to the file.
> If the file can be modified, then it is not read only.
It *is* read-only. But it can be changed and *then* modified.
> Let the VCSFS not return true from isReadOnly if one can get output
> stream and write to the file.
Do you mean, that I should lie????
You can not write to the file while it's read-only. The point is, that
at some point the file can be (but does not have to be) changed from
read-only mode to read-write.
I don't think, that I should return isReadOnly() == false when the
file *is* read-only. This can cause problems somewhere else.
Some better idea?
Two comments: The javadoc of isReadOnly describes that "the method
tests whether a file object can be written to". So regardless of the
actual state of the file on the disk, if it can be written (for
example in some automatic mode), it is not read only. I have improved
but please notice the line before my changes to say the same, so there
is no change in the meaning of the method. So please update your
filesystem to return false when in automatic mode.
But for the non-automatic case (which throws UserQuestionException),
we need the DataEditorSupport to really try to ask for the lock. I
suggest it to ask everytime user does first modification to the
document, but not report any exception if the file was read only.
Peter please make the change in CES.
I will resist a bit.
Such a solution is just an ugly hack.
Isn't it better to try to think some clean API solution? So we could
avoid this kind of regressions next time.
I believe the correct impl of markModified should never try to take
the lock of file. It is enough for CES to know whether the file is
read-only or not and behave accordingly, and take the lock, just at
the moment it is going to save the changes.
I don't like the method should be restricted this way. Such hacks
makes the class fragile, hard to maintain, and it leads to problems
like this one.
Do you think it is the correct way? I don't.
One more time passing to Yarda.
UserQuestionException is the API that was designed to allow
cooperation between (versioning) filesystems and editor. As far as I
know it used to work fine, even was not covered by test. That is why,
we did not catch the regression that was done in the CES. I agree that
should not happen again.
So I am asking the owner of this package, to rollback the regression,
write a test to check proper behaviour of UserQuestionException API
and then try to solve rollbacked commit in different way.
One more time passing this to Yarda, to get his answer.
I'm not interested in how to fix it since I can fix it easily, the
way, there will be called takeLock in markModified in all cases.(I
won't rollback fix of #24462 which was much worser bug).
But I want to point out the "design technique" I don't agree with.
Is that supposed to be an API contract between openapi and vcs that
support have to always call takeLock in its markModified method and
handle specially UserCancelException if thrown?
I claim that that is not a design, just a hack. How could somebody
looking at the code to know (in this case me) that the method call
takeLock is "holy" one and has some other meaning in markModified than
just taking the lock?
This kind of "design" leads to unmaintainable, ugly and fragile code.
I suppose if our goal is NB to be rock and solid, it can't contain
these kind of hacks.
"How could somebody looking at the code to know that the method call"
One could not, because the contract is not well documented nor tested.
That is a lesson we should learn from. Fix the documentation and next
time be more careful. Is there anything else I can do for you?
Well, it seems it doesn't have a sense.
Of course you could do something, and that is design a clear API.
I'm talking about that this case is bad design, just a hack.
Contract saying that "markModified method has to always call method
takeLock because when the UserCancelException is thrown from that one
then it is done this and that" is a clear nonsense.
Do you really want to have such sentences in javadoc?
The method markModified doesn't need any lock, so why it should be
restricted that way it tries to take it? Because there could be
somewhere and somewhen some vcs FS?
I agree with you just in that it is a lesson. But I think we are not
learning the right way.
OK, even I personaly don't agree with such a contract, I'm going to
put it in.
Fixed in [trunk]
Now the takeLock method is called from Env.markModified like it was
I tend to agree with Peter here, something is still wrong. At a
minimum, the Javadoc for FileObject.isReadOnly is inadequate. It says
that if it returns false, then getOutputStream etc. should succeed.
Fine. But it says nothing about when it returns true, implying (but
not saying) that in such cases getOutputStream etc. should fail. In
this case they probably will, but there is the further fact that
takeLock will throw UserQuestionException and permit the file to be
Therefore I don't think the newly reverted behavior of
DES.Env.markModified is wrong, but it is depending on a part of the
API which is not yet documented! (As Martin said, more or less.)
Suggest that FO.isRO Javadoc contain some additional information about
UQE and lock(). For example:
It is a good idea to call this method before attempting to perform any
operation on the FileObject that might throw an IOException simply
because it is read-only. If isReadOnly returns true, the operation may
be skipped, or the user notified that it cannot be done.
<em>However</em> it is often desirable for the user to be able to
continue the operation in case the filesystem supports making a file
writable. In this case calling code should:
<li>Call lock() and catch any exception thrown.
<li>If no exception is thrown, proceed with the operation.
<li>If a UserQuestionException is thrown, call confirmed() on it
(asynchronously - do not block any important threads). If true, return
to step 1. If false, exit. If an IOException is thrown, notify it and
<li>If another IOException is thrown, call isReadOnly(). If true,
ignore the exception (it is expected). If false, notify it. In either
Does that make more sense? Then Javadoc for DES.Env.markModified would
refer to FO.isReadOnly for a fuller explanation of its logic.
For sure, such javadoc should help. I'll put it in.
I applied the Jesse's suggestion:
Cool, works fine in RC1 build #200307092351 of NetBeans 3.5.1.