View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0011226 | MMW v4 | Framework: Scripts/Extensions | public | 2013-08-30 20:33 | 2013-10-05 02:51 |
| Reporter | zvezdan | Assigned To | |||
| Priority | high | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Fixed in Version | 4.1 | ||||
| Summary | 0011226: ISDBSongData::RenameByMask is moving file and changing Path property without UpdateDB | ||||
| Description | The RenameByMask method is moving file without used UpdateDB. Here is the test script: Option Explicit Sub OnStartUp() Dim oMenuItem Set oMenuItem = SDB.UI.AddMenuItem(SDB.UI.Menu_Pop_TrackList, 0, 0) oMenuItem.Caption = "Test changes to SongData" oMenuItem.OnClickFunc = "Test" oMenuItem.UseScript = Script.ScriptPath End Sub Sub Test(oMenuItem) Dim oSongData Dim bMoved Set oSongData = SDB.SelectedSongList.Item(0) ' oSongData.Path = "" On Error Resume Next bMoved = oSongData.RenameByMask(SDB.Tools.UFText2Mask _ ("C:\Music\<Genre>\<Album Artist>\<Album>\<Track#:2> - <Title>")) On Error GoTo 0 SDB.MessageBox bMoved & vbCr & oSongData.Path, mtInformation, Array(mbOK) oSongData.DiscardChanges End Sub If I remove the comment sign from the line: oSongData.Path = "" I get the error message: "Could not find file C, it will not be moved.". I tried that line because you have this strange thing with the Path property which says: "If you don't want to do so (to move file), you can set empty string to Path property first." That error message appears even with On Error Resume Next before problematic RenameByMask. Note that I am not using UpdateDB and even I have the new DiscardChanges method. Actuality, I want to get the string of path with applied some mask on some song, but without moving its file and even without changing its Path value in the database. | ||||
| Tags | No tags attached. | ||||
| Fixed in build | 1662 | ||||
|
|
If you don't want to change the current behavior of this method, then please add one new method, let say ISDBSongData::PathStringByMask, which could return the same string for path as RenameByMask, but without moving files. Here are some examples why I need this. The first one is my Export/Create Playlists script to which I want to add a possibility to replace paths in playlist files with some user-defined mask. For example, one track has database path c:\My Music\blah blah.mp3 and user want to export M3U file which should store the path for that file using e:\Music\<Genre>\<Artist>\<Title>.<Extension> mask. I cannot use the existing RenameByMask method for that because it moves files which I don't want at all. The another example would be my RegExp Find & Replace script to which I would like to add a possibility to replace paths using some user-defined mask. If you don't know how that script looks alike - it has one dialog box similar to Auto-Organize Files dialog box with one table displaying many files with their original (Old) and New paths. I cannot do this using the existing RenameByMask method because it moves files immediately, while I want just to display the new paths in the table first and to move files only if user clicks on the Replace button. |
|
|
Added the new ISDBSongData::PathByMask method as described/requested here: 0011121:0037561 Added in build 1659. |
|
|
Verified 1659 Left for Zvezdan to close it. |
|
|
I am getting the error #438 - Object doesn't support this property or method: 'oNewSong.PathByMask' with build 1661, where oNewSong is SongData object. By the way, you didn't update wiki about this method. |
|
|
Documented here: http://www.mediamonkey.com/wiki/index.php/ISDBSongData::PathByMask with working script attached. |
|
|
It works great, thank you very much! However, I need to ask, why did you make as a property, but not as a method? If it is a method it would be more consistent with the existing RenameByMask method. Besides, I cannot remember when I last saw (if at all) some property that is write-only. There are read-only or read/write, but write-only... |
|
|
Yes, it could be method like RenameByMask, but I personally felt it like write property (i.e. more similar to Path property). Not a big deal one way or the other IMHO. So letting it as property. EDIT: Also property is about "setting something" and method about "doing something", and in this case it just sets the Path, so I think that it is reasonable to left it as write property. |
|
|
I think it is more similar to RenameByMask method (with only difference that it takes effect only after UpdateDB), than it is similar to the Path property. The Path property is a "property" of SongData object as it is Artist or Album or Title, but I cannot see how "PathByMask" could be called a "property". Besides, if it is a method it could be used directly in some user-created VBScript expression that I am using in RegExp script. The write-only property will be much more complicated to use in such cases. |
|
|
Here are some excerpts from MSDN's page "Choosing Between Properties and Methods" at http://msdn.microsoft.com/en-us/library/vstudio/ms229054%28v=vs.100%29.aspx: "Properties are meant to be used like fields, meaning that properties should not be computationally complex or produce side effects." "Do use a property, rather than a method, if the value of the property is stored in the process memory and the property would just provide access to the value." "Do use a method, rather than a property, in the following situations. - The operation is orders of magnitude slower than a field set would be... - The operation is a conversion, ..." (which is obviously the case with the PathByMask) "- The operation returns a different result each time it is called, even if the parameters do not change..." (which is possible with the PathByMask, even if a mask do not change, since it depends of the Path value as well) "- The operation has a significant and observable side effect..." (with that logic even your implementation of Path that moves files is not correct, but we already talked about that.) |
|
|
Sorry, but I don't understand. 1. Why using S.PathByMask = "<artist> - <title>" will be much more complicated than using S.PathByMask("<artist> - <title>") ? 2. You wrote that operation is a conversion, I don't see a conversion, it only sets Path similar like S.Title = "Title" sets title. You are true that Path should be rather a method (because it also moves a file), but I still don't see why PathByMask should be a method if it only sets path. Nevertless it would be for me much less time consuming to change it from property to method than keeping this bizzare discussion with you. And because I created this property especially to satisfy your needs, I am going to do it again, but if you want it to change to method then it should be called rahter SetPathByMask(...) |
|
|
1. Was about something different than you replied 2. You are true that there is a string conversion I added new ISDBSongData::SetPathByMask method that does the same as the current PathByMask method, documented here: http://www.mediamonkey.com/wiki/index.php/ISDBSongData::SetPathByMask Added in 1662. |
|
|
1. It is not complicated for scripters, but from the program side of view. Do you want to say that: oSongData.PathByMask = ".\$First(<Genre>,1,1)\$MovePrefix(<Artist>)\$Replace(string,what,by)\...\whatever_you _could_write_using_masks" is not internally more complicated (i.e. computationally more complex) than: oSongData.Title = "some_simple_string"? 2. Do you really want to say that with the first example you are not doing internally any conversion? You are right that this discussion is bizarre when you could say that. |
|
|
I am sorry, I just posted slightly modified text before I saw your response. 1. You are right, I didn't know on what you mean by "complicated" and I will now try to explain. If you didn't see RegExp script - it has a possibility to execute user-defined VBScript expressions during run-time. For example, users could write UCase("$&") and script will convert field specified with Into combo box to upper case. Or they could write SDB.Tools.UFText2Mask("<Artist>") and script will assign Artist to the specified field (not matter how pointless is this example). So, users could use VBScript functions and MM property values quite easily in such expression. However, setting something to some property is not a function, but a statement. You cannot write oSongData.PathByMask = "something" in RegExp because this is not an expression, it does not return value, but sets value, instead of oSongData.PathByMask("something"). If you are using statement, then you should write something like Execute("oSongData.PathByMask = ""something"""), but then you have a problem with double quotes at best, and maybe something even more complicated. I could post you some expressions that I am using where I need to use 8 double quotes instead of just one because of using several nested Execute and Eval commands. Anyway, this new method, property or whatever you gave added, is quite welcome. Thank you again. |
|
|
Verified 1662 |
