Cat-log - open job logs in editor #2260
Conversation
|
This will be a really useful feature! Just taking a quick glance through before review: Something which would make it more convenient is if the editor command is run in the background in the case that the PS: There is a typo in |
hjoliver
changed the title
bin/cylc-cat-log
Outdated
|
|
||
| viewfile = NamedTemporaryFile( | ||
| suffix='.' + os.path.basename(filename), | ||
| prefix=suite + '.', dir=cylc_tmpdiir |
There was a problem hiding this comment.
typo (diir) - spotted by @oliver-sanders
bin/cylc-cat-log
Outdated
|
|
||
| for line in lines: | ||
| viewfile.write(line + '\n') | ||
| viewfile.seek(0, 0) |
There was a problem hiding this comment.
Indentation error - seek after all lines written, not after each.
bin/cylc-cat-log
Outdated
| 'WARNING: YOU HAVE EDITED A TEMPORARY READ_ONLY COPY OF: ') | ||
| print >> sys.stderr, filename | ||
| else: | ||
| print >> sys.stderr, 'NO CHANGES MADE TO: %s' % viewfile.name |
The reason we block on the editor command is to check and warn if the user edited and force-saved the file, which suggests they thought they were editing the original file rather than a temporary read-only copy. (This is more important in (most GUI editors - except for |
|
Made changes as suggested. |
bin/cylc-cat-log
Outdated
|
|
||
| viewfile = NamedTemporaryFile( | ||
| suffix='.' + os.path.basename(filename), | ||
| prefix=suite + '.', dir=cylc_tmpdir |
There was a problem hiding this comment.
One more problem found while testing: prefix=suite doesn't work for suite names containing / corresponding to the directory path under cylc-run/ - this makes the viewfile path refer to a non-existent directory. Can you change to this:
prefix=suite.replace('/','_') + '.' - and also do the same for the original code in cylc view.
bin/cylc-cat-log
Outdated
| @@ -296,6 +335,10 @@ def main(): | |||
| cmd_tmpl = str(GLOBAL_CFG.get_host_item( | |||
| "local tail command template")) | |||
| commands.append(shlex.split(cmd_tmpl % {"filename": filename})) | |||
| elif options.geditor or options.editor: | |||
| command_list = editor.split() | |||
There was a problem hiding this comment.
It would be safer to do this with shlex.split.
There was a problem hiding this comment.
@challurip - if you can make this change and push it up today, I'll merge this for the new release (tonight, if the anesthetic has worn off...)
cylc cat-log- allow opening job logs in users' editor ( temporary read only copy ); same via gcylc.[not ready for review yet - coming soon]