oleg-shilo / sublime-codemap

CodeMap - is a ST3 plugin for showing the code tree representing the code structure of the active view/document
MIT License
41 stars 4 forks source link

Issue with CodeMap caching system - Not capturing exceptions and not properly encoding files resulting in 0 byte files.. #32

Closed Acecool closed 6 years ago

Acecool commented 6 years ago

Priority: Low - The cache system you use is simply to load the Code - Map Panel with text on start so it isn't high priority...

I'm working on a caching system for XCodeMapper ( if the cached file timestamp is newer than the file AND the mapper - then display the cached file ) in order to speed up quickly switching between files.

I've been looking at file saving methods and there's a lot about encoding, ascii, utf-8, replacing or ignoring character issues, etc... I knew CodeMap had a file which was saved for session purposes so the panel didn't need to open in a newly opened SublimeText empty... so I looked at the "User\CodeMap\Code - Map" file and did some tests and noticed the file is almost always 0 bytes, and the "User\CodeMap\Code - Map.source" file which always specifies the last file saved and is accurate...

Because I am trying to save the same data and I'm coming up with encoding errors - I'm assuming the same thing is happening with CodeMap saving system...

This is likely to affect my files more so than standard because I allow special chars to be used... ( I just noticed it doesn't appear to save but only on exit which may be why it stays 0 bytes even between files ) Still, when removing the ƒ and ☼ chars but still end up with 0 byte file..

I'm submitting this because it still appears to be an issue - it may or may not be related ( I haven't dug that deep into CodeMap session saving system ) - but as I'm also working on this I have no problem submitting my completed load / save system with encoding functionality and full exception support to improve CodeMap and to ensure it always saves the session file...

This is my current system which still needs a bit of exception and conversion work to actually save the files other than 0 bytes ( plus support to delete the file if no text exists OR if the file ends up at 0 bytes ) - I'm including this in case you get to the CodeMap saving / loading system before I do so we both don't need to reinvent the same wheel - obviously most of this isn't useful to you, but it shows how I try to organize code and keep things combined and without repetition as much as possible.:

    ##
    ## Returns the timestamp from the actual file to help determine the last time it was modified...
    ##
    def GetFileTimeStamp( self, _file ):
        ## Default timestamp set at 0
        _timestamp = 0

        ## Attempt to read the file timestamp...
        try:
            ## We try because numerous issues can arise from file I / O
            _timestamp = os.path.getmtime( _file )
        except ( OSError ):
            ## File not found, inaccessible or other error - Set the timestamp to -1 so we know an error was thrown..
            _timestamp = -1

        ## Output the data..
        ## print( '>> Timestamp for file: ' + _file + ' is ' + str( _timestamp ) )

        ## Grab the real-time File TimeStamp if the file exists, otherwise -1 or 0...
        return _timestamp

    ##
    ## Returns the timestamp from the last time the file was processed, if ever. If not, it'll return 0..
    ##
    def GetCachedFileTimeStamp( self, _file ):
        ## Return the timestamp..
        return self.GetFileTimeStamp( self.GetCachePath( _file ) )

    ##
    ## Helper - Converts the full-path to and including the file-name to a single-filename which can be used as a filename saved...
    ##
    def GetSafeCachedFileName( self, _file ):
        ## Convert the filename to a safe-filename - will be using RegEx later..
        ## _file_safe = Acecool.string.Safe( _file ) + '.cache'

        ## Same as above except not using RegEx...
        _file_safe = _file
        _file_safe = _file_safe.replace( ':', '_' )
        _file_safe = _file_safe.replace( '\\', '_' )
        _file_safe = _file_safe.replace( '/', '_' )
        _file_safe = _file_safe.replace( ' ', '_' )
        _file_safe = _file_safe.replace( '__', '_' )

        ## Return the finally "safe for saving" filename..
        return _file_safe

    ##
    ## Helper - Returns the Cache directory by itself, or if the filename is provided then provide the full path with the converted file-name...
    ##
    def GetCachePath( self, _file = None ):
        ## If the filename is provided, we'll add that and convert it to the safe-string..
        if ( _file != None ):
            ## Returns %AppData%\Sublime Text 3\Packages\User\CodeMap\Cache\<SafeFileName>
            return os.path.join( sublime.packages_path( ), 'User', 'CodeMap', 'Cache', self.GetSafeCachedFileName( _file ) )
        else:
            ## Returns %AppData%\Sublime Text 3\Packages\User\CodeMap\Cache\
            return os.path.join( sublime.packages_path( ), 'User', 'CodeMap', 'Cache' )

    ##
    ## Helper - Saves the cache file..
    ##
    def SaveFileCache( self, _filename, _data = '' ):
        ## Grab the path to and including a safe-filename to save the cached file...
        _path = self.GetCachePath( _filename )

        ## Output the information..
        print( ' >> Saving CACHED FILE: ' + _filename + ' as ' + _path )

        ## Make sure we're not writing an empty file... I may allow this later...
        if ( _data != None and _data.strip( ) != '' ):
            ## Try, try, try again...
            try:
                ## Open the file with write-permissions and proper encoding
                with io.open( _path, 'w', encoding='utf8' ) as _file:
                    ## Write the data...
                    _file.write( _data )

                ## Once we're done, return None to ensure no error output is output to the output panel..
                return None
            except NameError as _error:
                ## Naming error? Too long maybe? Invalid chars? -- actually because global unicode didn't exist..
                ## return 'NameError( {0} ): {1} ' . format( _error.errno, _error.strerror )
                return 'XCodeMapper > SaveFileCache > NameError: ' + str( sys.exc_info( )[ 0 ] ) + ' / ' + str( _error )
            except UnicodeError as _error:
                ## Data Formatting Issue
                return 'XCodeMapper > SaveFileCache > UnicodeError( {0} ): {1} ' . format( str( _error.errno ), str( _error.strerror ) )
            except IOError as _error:
                ## General Input / Output Error
                return 'XCodeMapper > SaveFileCache > I / O Error( {0} ): {1} ' . format( str( _error.errno ), str( _error.strerror ) )
            except:
                # Handle all other errors
                return 'XCodeMapper > SaveFileCache > Unexpected error: ' + str( sys.exc_info( )[ 0 ] )

    ##
    ## Helper - Loads the cache file
    ##
    def LoadFileCache( self, _filename ):
        ## Grab the path to and including a safe-filename to save the cached file...
        _path = self.GetCachePath( _filename )

        ## Output the information
        print( ' >> Trying to LOAD CACHED File: ' + _filename + ' as ' + _path )

        ## Try to read the file data... This function should only be called if the file exists...
        try:
            ## Open the file with readonly-permissions and proper encoding
            with io.open( _path, 'r', encoding='utf8' ) as _file:
                ## Read the file and output it..
                return _file.read( )
        except:
            ## Output the error to the output window...
            return 'ExceptionThrown: ' + str( sys.exc_info( )[ 0 ] )

Note: Updated based on the below discovery... I haven't added support for all of the exceptions which can be thrown yet - that'll come soon but I can move on for now and add exception adding to the task list ( lower priority because a default catch exists with output of the error and error catching for other types exists too )...

My caching system is alive :-)

Acecool commented 6 years ago

Turns out io is just the thing we both need:


                ## Open the file with write-permissions and proper encoding
                with io.open( _path, 'w', encoding='utf8' ) as _file:
                    ## Write the data...
                    _file.write( _data )

            ## Open the file with readonly-permissions and proper encoding
            with io.open( _path, 'r', encoding='utf8' ) as _file:
                ## Read the file and output it..
                return _file.read( )

_data being what I submit through... I updated the code in the first post!

mg979 commented 6 years ago

Now, since you've started contributing I'd like to point out a couple of things... I have the impression that you go too far ahead with assumptions you make minute after minute, and start writing tons of stuff, and that makes everything difficult. Before writing and 'patching' things, maybe you could ask why some things are as they are, and state your desired behaviour, but without writing MB of text and code before even doing that. Here for example, the map has a physical file, but the map itself is not written to a file. There is no 'map saving system' as far as I know. The file is used to check if the map is open or not, that's it.

I'm working on a caching system for XCodeMapper ( if the cached file timestamp is newer than the file AND the mapper - then display the cached file ) in order to speed up quickly switching between files.

Speed up? If I'm understanding well, you're talking about the delay that you see between file switching and code map update. If this is the case, it happens because map generation is asynchronous, it's not an issue. It could be done synchronous but it could create problems.

Also, I don't know what @oleg-shilo has to say, but I'd prefer if the code base remained within acceptable levels of bloat(that is as little as possible for me). I say this because the stuff you do (both normal text and code snippets you post) is generally kilometric... Maybe it's me, but consider that if more people work on the code, if things get overgrown (for little or no reasons) nobody will understand anything anymore. Just my opinion on this.

mg979 commented 6 years ago

Also, the map is small and it should be generated very fast, I mean in milliseconds. If it takes longer, there is is some script generation problem. In that case it would need 'fixing', but it would never need 'caching', if you understand what I mean.

oleg-shilo commented 6 years ago

@mg979, I absolutely back your two posts above. I merged @Acecool's first PR a day ago. But even though in this case it was a simple straightforward change I felt a bit uneasy. To be honest, I don't have full confidence that it may not affect something else.

Anyway, I outlined the way @Acecool work should be integrated with CodeMap (my last post in this huge thread).

To put it simple, mappers are not part of CodeMap but rather separate external products. CodeMap as a mappers hosting solution has already reached required/anticipated level of functionality and does what it supposed to do. Thus it is fully (saturated functionality wise) and the only further changes to be are true fixes or truly necessary (e.g. initially overlooked) functionality that addresses real issues.

Acecool commented 6 years ago

I agree that the mappers are external - my only suggestion was if we ended up merging some of the XCM features ( Category system and easy adding of syntax with various search methods, etc ) to CodeMap - ie the back-end -- should we include the mappers ( the collections of AddSyntax and modifications to the data found, syntax error checking, etc.. ) to the custom_mappers folder ( if we got to a point where they could be supported ).. I never meant to imply to set the mappers up in the back-end because that'd close down the system and make it difficult to modify, etc....

I write incredibly fast - I have to...

This post was because I previously believed to see the "Code - Map" file populated with a cache of the data which is output... I may have been mistaken - I looked at the code and saw with open(code_map_file+'.source', "w") as file: writing data, but I didn't see with open(code_map_file, "w") as file: writing an empty file... so this can be closed - and I'll close it... I submitted this post without thoroughly looking through everything because I thought I saw the data cached in that file, or some other file, but it looks as though I may be wrong in that regards...

If I make a mistake, just let me know! - I am human, it does happen...

Also, if I make a submission and you're not comfortable with something - let me know to change it or edit it.

I brought up the subject of merging again because you mentioned it was ok to post the XCodeMapper features / etc.. and changelog post and said that it was what you envisioned CodeMap being ( I thought you meant for the back-end the category system and those features )... If I got that wrong, I'm sorry...

When I post an error or issue - and I post a large chunk of code I'm working on which is related - it's simply to show a possibly solution or what's missing ( ie not outputting exception for release-products doesn't let the user or the dev know what the problems are therefore they can't report them to you )... It's not to intimidate or cause any other issues.